From 9771bf6b1b7033220ca6fc0c6adf6025e6b5d122 Mon Sep 17 00:00:00 2001 From: Jon Jensen Date: Tue, 12 Jul 2022 16:55:54 -0600 Subject: [PATCH] feat: accept registry-scoped certfile and keyfile as credentials Closes #4765 RFC: https://github.com/npm/rfcs/pull/591 While this doesn't directly allow top-level cert/key as credentials (per the original issue), it's a more targeted/secure approach that accomplishes the same end-result; the new options are scoped to a specific registry, and the actual cert/key contents are much less likely to be exposed. See the RFC for more context. Depends on: * https://github.com/npm/npm-registry-fetch/pull/125 * https://github.com/npm/config/pull/69 --- docs/content/using-npm/config.md | 8 +++-- lib/commands/publish.js | 2 +- lib/utils/config/definitions.js | 7 +++-- lib/utils/get-identity.js | 4 +-- .../test/lib/commands/publish.js.test.cjs | 6 +++- .../lib/utils/config/definitions.js.test.cjs | 8 +++-- .../lib/utils/config/describe-all.js.test.cjs | 8 +++-- test/lib/commands/publish.js | 31 ++++++++++++++++++- test/lib/commands/whoami.js | 14 +++++++++ 9 files changed, 71 insertions(+), 17 deletions(-) diff --git a/docs/content/using-npm/config.md b/docs/content/using-npm/config.md index ffbf9be055719..e3e1bd6c73bb3 100644 --- a/docs/content/using-npm/config.md +++ b/docs/content/using-npm/config.md @@ -357,8 +357,9 @@ newlines replaced by the string "\n". For example: cert="-----BEGIN CERTIFICATE-----\nXXXX\nXXXX\n-----END CERTIFICATE-----" ``` -It is _not_ the path to a certificate file (and there is no "certfile" -option). +It is _not_ the path to a certificate file, though you can set a +registry-scoped "certfile" path like +"//other-registry.tld/:certfile=/path/to/cert.pem". @@ -946,7 +947,8 @@ format with newlines replaced by the string "\n". For example: key="-----BEGIN PRIVATE KEY-----\nXXXX\nXXXX\n-----END PRIVATE KEY-----" ``` -It is _not_ the path to a key file (and there is no "keyfile" option). +It is _not_ the path to a key file, though you can set a registry-scoped +"keyfile" path like "//other-registry.tld/:keyfile=/path/to/key.pem". diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 579f5d6e74e67..0840346a7fa1d 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -101,7 +101,7 @@ class Publish extends BaseCommand { const resolved = npa.resolve(manifest.name, manifest.version) const registry = npmFetch.pickRegistry(resolved, opts) const creds = this.npm.config.getCredentialsByURI(registry) - const noCreds = !creds.token && !creds.username + const noCreds = !(creds.token || creds.username || creds.certfile && creds.keyfile) const outputRegistry = replaceInfo(registry) if (noCreds) { diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js index 665ed1efe5e61..7d6af2473f2bd 100644 --- a/lib/utils/config/definitions.js +++ b/lib/utils/config/definitions.js @@ -436,8 +436,8 @@ define('cert', { cert="-----BEGIN CERTIFICATE-----\\nXXXX\\nXXXX\\n-----END CERTIFICATE-----" \`\`\` - It is _not_ the path to a certificate file (and there is no "certfile" - option). + It is _not_ the path to a certificate file, though you can set a registry-scoped + "certfile" path like "//other-registry.tld/:certfile=/path/to/cert.pem". `, flatten, }) @@ -1118,7 +1118,8 @@ define('key', { key="-----BEGIN PRIVATE KEY-----\\nXXXX\\nXXXX\\n-----END PRIVATE KEY-----" \`\`\` - It is _not_ the path to a key file (and there is no "keyfile" option). + It is _not_ the path to a key file, though you can set a registry-scoped + "keyfile" path like "//other-registry.tld/:keyfile=/path/to/key.pem". `, flatten, }) diff --git a/lib/utils/get-identity.js b/lib/utils/get-identity.js index f4aedb89b3957..41d882473ab7b 100644 --- a/lib/utils/get-identity.js +++ b/lib/utils/get-identity.js @@ -9,8 +9,8 @@ module.exports = async (npm, opts) => { return creds.username } - // No username, but we have a token; fetch the username from registry - if (creds.token) { + // No username, but we have other credentials; fetch the username from registry + if (creds.token || creds.certfile && creds.keyfile) { const registryData = await npmFetch.json('/-/whoami', { ...opts }) return registryData.username } diff --git a/tap-snapshots/test/lib/commands/publish.js.test.cjs b/tap-snapshots/test/lib/commands/publish.js.test.cjs index f90cf3152e80f..d85a1164e22bf 100644 --- a/tap-snapshots/test/lib/commands/publish.js.test.cjs +++ b/tap-snapshots/test/lib/commands/publish.js.test.cjs @@ -56,7 +56,11 @@ Array [ ] ` -exports[`test/lib/commands/publish.js TAP has auth for scope configured registry > new package version 1`] = ` +exports[`test/lib/commands/publish.js TAP has mTLS auth for scope configured registry > new package version 1`] = ` ++ @npm/test-package@1.0.0 +` + +exports[`test/lib/commands/publish.js TAP has token auth for scope configured registry > new package version 1`] = ` + @npm/test-package@1.0.0 ` diff --git a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs index 04d304a2254d9..89c9969d69424 100644 --- a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs +++ b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs @@ -404,8 +404,9 @@ newlines replaced by the string "\\n". For example: cert="-----BEGIN CERTIFICATE-----\\nXXXX\\nXXXX\\n-----END CERTIFICATE-----" \`\`\` -It is _not_ the path to a certificate file (and there is no "certfile" -option). +It is _not_ the path to a certificate file, though you can set a +registry-scoped "certfile" path like +"//other-registry.tld/:certfile=/path/to/cert.pem". ` exports[`test/lib/utils/config/definitions.js TAP > config description for ci-name 1`] = ` @@ -1016,7 +1017,8 @@ format with newlines replaced by the string "\\n". For example: key="-----BEGIN PRIVATE KEY-----\\nXXXX\\nXXXX\\n-----END PRIVATE KEY-----" \`\`\` -It is _not_ the path to a key file (and there is no "keyfile" option). +It is _not_ the path to a key file, though you can set a registry-scoped +"keyfile" path like "//other-registry.tld/:keyfile=/path/to/key.pem". ` exports[`test/lib/utils/config/definitions.js TAP > config description for legacy-bundling 1`] = ` diff --git a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs index a291af6dedfae..a9247f49c0418 100644 --- a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs +++ b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs @@ -230,8 +230,9 @@ newlines replaced by the string "\\n". For example: cert="-----BEGIN CERTIFICATE-----\\nXXXX\\nXXXX\\n-----END CERTIFICATE-----" \`\`\` -It is _not_ the path to a certificate file (and there is no "certfile" -option). +It is _not_ the path to a certificate file, though you can set a +registry-scoped "certfile" path like +"//other-registry.tld/:certfile=/path/to/cert.pem". @@ -819,7 +820,8 @@ format with newlines replaced by the string "\\n". For example: key="-----BEGIN PRIVATE KEY-----\\nXXXX\\nXXXX\\n-----END PRIVATE KEY-----" \`\`\` -It is _not_ the path to a key file (and there is no "keyfile" option). +It is _not_ the path to a key file, though you can set a registry-scoped +"keyfile" path like "//other-registry.tld/:keyfile=/path/to/key.pem". diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 3cbe962382e21..16b79df532d82 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -327,7 +327,7 @@ t.test('no auth for scope configured registry', async t => { ) }) -t.test('has auth for scope configured registry', async t => { +t.test('has token auth for scope configured registry', async t => { const spec = npa('@npm/test-package') const { npm, joinedOutput } = await loadMockNpm(t, { config: { @@ -356,6 +356,35 @@ t.test('has auth for scope configured registry', async t => { t.matchSnapshot(joinedOutput(), 'new package version') }) +t.test('has mTLS auth for scope configured registry', async t => { + const spec = npa('@npm/test-package') + const { npm, joinedOutput } = await loadMockNpm(t, { + config: { + '@npm:registry': alternateRegistry, + [`${alternateRegistry.slice(6)}/:certfile`]: '/some.cert', + [`${alternateRegistry.slice(6)}/:keyfile`]: '/some.key', + }, + prefixDir: { + 'package.json': JSON.stringify({ + name: '@npm/test-package', + version: '1.0.0', + }, null, 2), + }, + globals: ({ prefix }) => ({ + 'process.cwd': () => prefix, + }), + }) + const registry = new MockRegistry({ + tap: t, + registry: alternateRegistry, + }) + registry.nock.put(`/${spec.escapedName}`, body => { + return t.match(body, { name: '@npm/test-package' }) + }).reply(200, {}) + await npm.exec('publish', []) + t.matchSnapshot(joinedOutput(), 'new package version') +}) + t.test('workspaces', t => { const dir = { 'package.json': JSON.stringify( diff --git a/test/lib/commands/whoami.js b/test/lib/commands/whoami.js index ad7c223888df4..d63b49015f0d0 100644 --- a/test/lib/commands/whoami.js +++ b/test/lib/commands/whoami.js @@ -34,6 +34,20 @@ t.test('npm whoami --json', async t => { t.equal(JSON.parse(joinedOutput()), username, 'should print username') }) +t.test('npm whoami using mTLS', async t => { + const { npm, joinedOutput } = await loadMockNpm(t, { config: { + '//registry.npmjs.org/:certfile': '/some.cert', + '//registry.npmjs.org/:keyfile': '/some.key', + } }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + registry.whoami({ username }) + await npm.exec('whoami', []) + t.equal(joinedOutput(), username, 'should print username') +}) + t.test('credentials from token', async t => { const { npm, joinedOutput } = await loadMockNpm(t, { config: {