Skip to content

Commit 6b00d12

Browse files
mikejancaralan-agius4
mikejancar
authored and
alan-agius4
committed
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.
1 parent 02cc9c0 commit 6b00d12

File tree

6 files changed

+150
-62
lines changed

6 files changed

+150
-62
lines changed

‎packages/angular/cli/utilities/package-metadata.ts

Lines changed: 76 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ function readOptions(
131131
logger.info(`Locating potential ${baseFilename} files:`);
132132
}
133133

134-
const options: PackageManagerOptions = {};
134+
let options: PackageManagerOptions = {};
135135
for (const location of [...defaultConfigLocations, ...projectConfigLocations]) {
136136
if (existsSync(location)) {
137137
if (showPotentials) {
@@ -142,58 +142,84 @@ function readOptions(
142142
// Normalize RC options that are needed by 'npm-registry-fetch'.
143143
// See: https://github.com/npm/npm-registry-fetch/blob/ebddbe78a5f67118c1f7af2e02c8a22bcaf9e850/index.js#L99-L126
144144
const rcConfig: PackageManagerOptions = yarn ? lockfile.parse(data) : ini.parse(data);
145-
for (const [key, value] of Object.entries(rcConfig)) {
146-
let substitutedValue = value;
147145

148-
// Substitute any environment variable references.
149-
if (typeof value === 'string') {
150-
substitutedValue = value.replace(/\$\{([^\}]+)\}/, (_, name) => process.env[name] || '');
151-
}
146+
options = normalizeOptions(rcConfig, location);
147+
}
148+
}
149+
150+
for (const [key, value] of Object.entries(process.env)) {
151+
if (!value || !key.toLowerCase().startsWith('npm_config_')) {
152+
continue;
153+
}
154+
155+
const normalizedName = key
156+
.substr(11)
157+
.replace(/(?!^)_/g, '-') // don't replace _ at the start of the key
158+
.toLowerCase();
159+
options[normalizedName] = value;
160+
}
152161

153-
switch (key) {
154-
// Unless auth options are scope with the registry url it appears that npm-registry-fetch ignores them,
155-
// even though they are documented.
156-
// https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/README.md
157-
// https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/auth.js#L45-L91
158-
case '_authToken':
159-
case 'token':
160-
case 'username':
161-
case 'password':
162-
case '_auth':
163-
case 'auth':
164-
options['forceAuth'] ??= {};
165-
options['forceAuth'][key] = substitutedValue;
166-
break;
167-
case 'noproxy':
168-
case 'no-proxy':
169-
options['noProxy'] = substitutedValue;
170-
break;
171-
case 'maxsockets':
172-
options['maxSockets'] = substitutedValue;
173-
break;
174-
case 'https-proxy':
175-
case 'proxy':
176-
options['proxy'] = substitutedValue;
177-
break;
178-
case 'strict-ssl':
179-
options['strictSSL'] = substitutedValue;
180-
break;
181-
case 'local-address':
182-
options['localAddress'] = substitutedValue;
183-
break;
184-
case 'cafile':
185-
if (typeof substitutedValue === 'string') {
186-
const cafile = path.resolve(path.dirname(location), substitutedValue);
187-
try {
188-
options['ca'] = readFileSync(cafile, 'utf8').replace(/\r?\n/g, '\n');
189-
} catch {}
190-
}
191-
break;
192-
default:
193-
options[key] = substitutedValue;
194-
break;
162+
options = normalizeOptions(options);
163+
164+
return options;
165+
}
166+
167+
function normalizeOptions(
168+
rawOptions: PackageManagerOptions,
169+
location = process.cwd(),
170+
): PackageManagerOptions {
171+
const options: PackageManagerOptions = {};
172+
173+
for (const [key, value] of Object.entries(rawOptions)) {
174+
let substitutedValue = value;
175+
176+
// Substitute any environment variable references.
177+
if (typeof value === 'string') {
178+
substitutedValue = value.replace(/\$\{([^\}]+)\}/, (_, name) => process.env[name] || '');
179+
}
180+
181+
switch (key) {
182+
// Unless auth options are scope with the registry url it appears that npm-registry-fetch ignores them,
183+
// even though they are documented.
184+
// https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/README.md
185+
// https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/auth.js#L45-L91
186+
case '_authToken':
187+
case 'token':
188+
case 'username':
189+
case 'password':
190+
case '_auth':
191+
case 'auth':
192+
options['forceAuth'] ??= {};
193+
options['forceAuth'][key] = substitutedValue;
194+
break;
195+
case 'noproxy':
196+
case 'no-proxy':
197+
options['noProxy'] = substitutedValue;
198+
break;
199+
case 'maxsockets':
200+
options['maxSockets'] = substitutedValue;
201+
break;
202+
case 'https-proxy':
203+
case 'proxy':
204+
options['proxy'] = substitutedValue;
205+
break;
206+
case 'strict-ssl':
207+
options['strictSSL'] = substitutedValue;
208+
break;
209+
case 'local-address':
210+
options['localAddress'] = substitutedValue;
211+
break;
212+
case 'cafile':
213+
if (typeof substitutedValue === 'string') {
214+
const cafile = path.resolve(path.dirname(location), substitutedValue);
215+
try {
216+
options['ca'] = readFileSync(cafile, 'utf8').replace(/\r?\n/g, '\n');
217+
} catch {}
195218
}
196-
}
219+
break;
220+
default:
221+
options[key] = substitutedValue;
222+
break;
197223
}
198224
}
199225

‎tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { expectFileNotToExist, expectFileToExist } from '../../../utils/fs';
2+
import { getActivePackageManager } from '../../../utils/packages';
3+
import { git, ng } from '../../../utils/process';
4+
import {
5+
createNpmConfigForAuthentication,
6+
setNpmEnvVarsForAuthentication,
7+
} from '../../../utils/registry';
8+
9+
export default async function () {
10+
const packageManager = getActivePackageManager();
11+
12+
if (packageManager === 'npm') {
13+
const originalEnvironment = { ...process.env };
14+
try {
15+
const command = ['add', '@angular/pwa', '--skip-confirmation'];
16+
17+
// Environment variables only
18+
await expectFileNotToExist('src/manifest.webmanifest');
19+
setNpmEnvVarsForAuthentication();
20+
await ng(...command);
21+
await expectFileToExist('src/manifest.webmanifest');
22+
await git('clean', '-dxf');
23+
24+
// Mix of config file and env vars works
25+
await expectFileNotToExist('src/manifest.webmanifest');
26+
await createNpmConfigForAuthentication(false, true);
27+
await ng(...command);
28+
await expectFileToExist('src/manifest.webmanifest');
29+
} finally {
30+
process.env = originalEnvironment;
31+
}
32+
}
33+
}

‎tests/legacy-cli/e2e/tests/commands/add/registry-option.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,20 @@ export default async function () {
1111
'.npmrc': 'registry=http://127.0.0.1:9999',
1212
});
1313
// The environment variable has priority over the .npmrc
14-
const originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
15-
delete process.env['NPM_CONFIG_REGISTRY'];
14+
let originalRegistryVariable;
15+
if (process.env['NPM_CONFIG_REGISTRY']) {
16+
originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
17+
delete process.env['NPM_CONFIG_REGISTRY'];
18+
}
1619

1720
try {
1821
await expectToFail(() => ng('add', '@angular/pwa', '--skip-confirmation'));
1922

2023
await ng('add', `--registry=${testRegistry}`, '@angular/pwa', '--skip-confirmation');
2124
await expectFileToExist('src/manifest.webmanifest');
2225
} finally {
23-
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
26+
if (originalRegistryVariable) {
27+
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
28+
}
2429
}
2530
}

‎tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ import { expectToFail } from '../../../utils/utils';
55

66
export default async function () {
77
// The environment variable has priority over the .npmrc
8-
const originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
9-
delete process.env['NPM_CONFIG_REGISTRY'];
8+
let originalRegistryVariable;
9+
if (process.env['NPM_CONFIG_REGISTRY']) {
10+
originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
11+
delete process.env['NPM_CONFIG_REGISTRY'];
12+
}
1013

1114
try {
1215
const command = ['add', '@angular/pwa', '--skip-confirmation'];
@@ -32,6 +35,8 @@ export default async function () {
3235
await createNpmConfigForAuthentication(true, true);
3336
await expectToFail(() => ng(...command));
3437
} finally {
35-
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
38+
if (originalRegistryVariable) {
39+
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
40+
}
3641
}
3742
}

‎tests/legacy-cli/e2e/tests/update/update-secure-registry.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@ import { expectToFail } from '../../utils/utils';
44

55
export default async function () {
66
// The environment variable has priority over the .npmrc
7-
const originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
8-
delete process.env['NPM_CONFIG_REGISTRY'];
7+
let originalRegistryVariable;
8+
if (process.env['NPM_CONFIG_REGISTRY']) {
9+
originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
10+
delete process.env['NPM_CONFIG_REGISTRY'];
11+
}
912

1013
const worksMessage = 'We analyzed your package.json';
1114

@@ -30,6 +33,8 @@ export default async function () {
3033
await createNpmConfigForAuthentication(true, true);
3134
await expectToFail(() => ng('update'));
3235
} finally {
33-
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
36+
if (originalRegistryVariable) {
37+
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
38+
}
3439
}
3540
}

‎tests/legacy-cli/e2e/utils/registry.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ export function createNpmRegistry(withAuthentication = false): ChildProcess {
1919
});
2020
}
2121

22+
// Token was generated using `echo -n 'testing:s3cret' | openssl base64`.
23+
const VALID_TOKEN = `dGVzdGluZzpzM2NyZXQ=`;
24+
const SECURE_REGISTRY = `//localhost:4876/`;
25+
2226
export function createNpmConfigForAuthentication(
2327
/**
2428
* When true, the authentication token will be scoped to the registry URL.
@@ -37,9 +41,8 @@ export function createNpmConfigForAuthentication(
3741
/** When true, an incorrect token is used. Use this to validate authentication failures. */
3842
invalidToken = false,
3943
): Promise<void> {
40-
// Token was generated using `echo -n 'testing:s3cret' | openssl base64`.
41-
const token = invalidToken ? `invalid=` : `dGVzdGluZzpzM2NyZXQ=`;
42-
const registry = `//localhost:4876/`;
44+
const token = invalidToken ? `invalid=` : VALID_TOKEN;
45+
const registry = SECURE_REGISTRY;
4346

4447
return writeFile(
4548
'.npmrc',
@@ -54,3 +57,14 @@ export function createNpmConfigForAuthentication(
5457
`,
5558
);
5659
}
60+
61+
export function setNpmEnvVarsForAuthentication(
62+
/** When true, an incorrect token is used. Use this to validate authentication failures. */
63+
invalidToken = false,
64+
): void {
65+
const token = invalidToken ? `invalid=` : VALID_TOKEN;
66+
const registry = SECURE_REGISTRY;
67+
68+
process.env['NPM_CONFIG_REGISTRY'] = `http:${registry}`;
69+
process.env['NPM_CONFIG__AUTH'] = token;
70+
}

0 commit comments

Comments
 (0)