Skip to content

Commit

Permalink
feat: accept registry-scoped certfile and keyfile as credentials (#5160)
Browse files Browse the repository at this point in the history
Closes #4765
RFC: npm/rfcs#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:
* npm/npm-registry-fetch#125
* npm/config#69
  • Loading branch information
jenseng authored Jul 20, 2022
1 parent 51b12a0 commit 5ef53ee
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 17 deletions.
8 changes: 5 additions & 3 deletions docs/content/using-npm/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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".

<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
Expand Down Expand Up @@ -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".

<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 4 additions & 3 deletions lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
})
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/get-identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion tap-snapshots/test/lib/commands/publish.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected]
`

exports[`test/lib/commands/publish.js TAP has token auth for scope configured registry > new package version 1`] = `
+ @npm/[email protected]
`

Expand Down
8 changes: 5 additions & 3 deletions tap-snapshots/test/lib/utils/config/definitions.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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`] = `
Expand Down Expand Up @@ -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`] = `
Expand Down
8 changes: 5 additions & 3 deletions tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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".
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
Expand Down Expand Up @@ -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".
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
Expand Down
31 changes: 30 additions & 1 deletion test/lib/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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(
Expand Down
14 changes: 14 additions & 0 deletions test/lib/commands/whoami.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down

0 comments on commit 5ef53ee

Please sign in to comment.