From 1b0b9ea85795923509870ff8cec94ebdd2eb9000 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 --- lib/commands/publish.js | 3 ++- lib/utils/config/definitions.js | 7 ++++--- lib/utils/get-identity.js | 5 +++-- lib/utils/has-credentials.js | 5 +++++ .../test/lib/utils/config/definitions.js.test.cjs | 8 +++++--- .../test/lib/utils/config/describe-all.js.test.cjs | 8 +++++--- 6 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 lib/utils/has-credentials.js diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 579f5d6e74e67..4cc3333bcce71 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -7,6 +7,7 @@ const runScript = require('@npmcli/run-script') const pacote = require('pacote') const npa = require('npm-package-arg') const npmFetch = require('npm-registry-fetch') +const hasCredentials = require('../utils/has-credentials.js') const replaceInfo = require('../utils/replace-info.js') const otplease = require('../utils/otplease.js') @@ -101,7 +102,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 = !hasCredentials(creds) 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..51586457626e5 100644 --- a/lib/utils/get-identity.js +++ b/lib/utils/get-identity.js @@ -1,4 +1,5 @@ const npmFetch = require('npm-registry-fetch') +const hasCredentials = require('./has-credentials.js') module.exports = async (npm, opts) => { const { registry } = opts @@ -9,8 +10,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 (hasCredentials(creds)) { const registryData = await npmFetch.json('/-/whoami', { ...opts }) return registryData.username } diff --git a/lib/utils/has-credentials.js b/lib/utils/has-credentials.js new file mode 100644 index 0000000000000..bbd69595e6a57 --- /dev/null +++ b/lib/utils/has-credentials.js @@ -0,0 +1,5 @@ +function hasCredentials (creds) { + return !!(creds.token || creds.username || creds.certfile && creds.keyfile) +} + +module.exports = hasCredentials 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".