From 978c616652b31ffb31dc85c0bfffdf3abaf0c7de Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 10 Oct 2023 10:11:17 -0700 Subject: [PATCH 1/5] deps: npm-registry-fetch@16.1.0 --- node_modules/npm-registry-fetch/lib/auth.js | 64 +++++++++++++++----- node_modules/npm-registry-fetch/lib/index.js | 2 + node_modules/npm-registry-fetch/package.json | 14 ++--- package-lock.json | 8 +-- package.json | 2 +- 5 files changed, 61 insertions(+), 29 deletions(-) diff --git a/node_modules/npm-registry-fetch/lib/auth.js b/node_modules/npm-registry-fetch/lib/auth.js index 870ce0d923cd0..9270025fa8d90 100644 --- a/node_modules/npm-registry-fetch/lib/auth.js +++ b/node_modules/npm-registry-fetch/lib/auth.js @@ -4,8 +4,8 @@ const npa = require('npm-package-arg') const { URL } = require('url') // Find the longest registry key that is used for some kind of auth -// in the options. -const regKeyFromURI = (uri, opts) => { +// in the options. Returns the registry key and the auth config. +const regFromURI = (uri, opts) => { const parsed = new URL(uri) // try to find a config key indicating we have auth for this registry // can be one of :_authToken, :_auth, :_password and :username, or @@ -14,23 +14,40 @@ const regKeyFromURI = (uri, opts) => { // stopping when we reach '//'. let regKey = `//${parsed.host}${parsed.pathname}` while (regKey.length > '//'.length) { + const authKey = hasAuth(regKey, opts) // got some auth for this URI - if (hasAuth(regKey, opts)) { - return regKey + if (authKey) { + return { regKey, authKey } } // can be either //host/some/path/:_auth or //host/some/path:_auth // walk up by removing EITHER what's after the slash OR the slash itself regKey = regKey.replace(/([^/]+|\/)$/, '') } + return { regKey: false, authKey: null } } -const hasAuth = (regKey, opts) => ( - opts[`${regKey}:_authToken`] || - opts[`${regKey}:_auth`] || - opts[`${regKey}:username`] && opts[`${regKey}:_password`] || - opts[`${regKey}:certfile`] && opts[`${regKey}:keyfile`] -) +// Not only do we want to know if there is auth, but if we are calling `npm +// logout` we want to know what config value specifically provided it. This is +// so we can look up where the config came from to delete it (i.e. user vs +// project) +const hasAuth = (regKey, opts) => { + if (opts[`${regKey}:_authToken`]) { + return '_authToken' + } + if (opts[`${regKey}:_auth`]) { + return '_auth' + } + if (opts[`${regKey}:username`] && opts[`${regKey}:_password`]) { + // 'password' can be inferred to also be present + return 'username' + } + if (opts[`${regKey}:certfile`] && opts[`${regKey}:keyfile`]) { + // 'keyfile' can be inferred to also be present + return 'certfile' + } + return false +} const sameHost = (a, b) => { const parsedA = new URL(a) @@ -63,11 +80,14 @@ const getAuth = (uri, opts = {}) => { if (!uri) { throw new Error('URI is required') } - const regKey = regKeyFromURI(uri, forceAuth || opts) + const { regKey, authKey } = regFromURI(uri, forceAuth || opts) // we are only allowed to use what's in forceAuth if specified if (forceAuth && !regKey) { return new Auth({ + // if we force auth we don't want to refer back to anything in config + regKey: false, + authKey: null, scopeAuthKey: null, token: forceAuth._authToken || forceAuth.token, username: forceAuth.username, @@ -88,8 +108,8 @@ const getAuth = (uri, opts = {}) => { // registry where we logged in, but the same auth SHOULD be sent // to that artifact host, then we track where it was coming in from, // and warn the user if we get a 4xx error on it. - const scopeAuthKey = regKeyFromURI(registry, opts) - return new Auth({ scopeAuthKey }) + const { regKey: scopeAuthKey, authKey: _authKey } = regFromURI(registry, opts) + return new Auth({ scopeAuthKey, regKey: scopeAuthKey, authKey: _authKey }) } } @@ -104,6 +124,8 @@ const getAuth = (uri, opts = {}) => { return new Auth({ scopeAuthKey: null, + regKey, + authKey, token, auth, username, @@ -114,8 +136,22 @@ const getAuth = (uri, opts = {}) => { } class Auth { - constructor ({ token, auth, username, password, scopeAuthKey, certfile, keyfile }) { + constructor ({ + token, + auth, + username, + password, + scopeAuthKey, + certfile, + keyfile, + regKey, + authKey, + }) { + // same as regKey but only present for scoped auth. Should have been named scopeRegKey this.scopeAuthKey = scopeAuthKey + // `${regKey}:${authKey}` will get you back to the auth config that gave us auth + this.regKey = regKey + this.authKey = authKey this.token = null this.auth = null this.isBasicAuth = false diff --git a/node_modules/npm-registry-fetch/lib/index.js b/node_modules/npm-registry-fetch/lib/index.js index 23e349c5c5b96..bb413f862d92d 100644 --- a/node_modules/npm-registry-fetch/lib/index.js +++ b/node_modules/npm-registry-fetch/lib/index.js @@ -166,6 +166,8 @@ function regFetch (uri, /* istanbul ignore next */ opts_ = {}) { return Promise.resolve(body).then(doFetch) } +module.exports.getAuth = getAuth + module.exports.json = fetchJSON function fetchJSON (uri, opts) { return regFetch(uri, opts).then(res => res.json()) diff --git a/node_modules/npm-registry-fetch/package.json b/node_modules/npm-registry-fetch/package.json index 2afadf939743b..b715d52391a93 100644 --- a/node_modules/npm-registry-fetch/package.json +++ b/node_modules/npm-registry-fetch/package.json @@ -1,6 +1,6 @@ { "name": "npm-registry-fetch", - "version": "16.0.0", + "version": "16.1.0", "description": "Fetch-based http client for use with npm registry APIs", "main": "lib", "files": [ @@ -41,7 +41,7 @@ }, "devDependencies": { "@npmcli/eslint-config": "^4.0.0", - "@npmcli/template-oss": "4.18.0", + "@npmcli/template-oss": "4.19.0", "cacache": "^18.0.0", "nock": "^13.2.4", "require-inject": "^1.4.4", @@ -61,13 +61,7 @@ }, "templateOSS": { "//@npmcli/template-oss": "This file is partially managed by @npmcli/template-oss. Edits may be overwritten.", - "version": "4.18.0", - "publish": "true", - "ciVersions": [ - "16.14.0", - "16.x", - "18.0.0", - "18.x" - ] + "version": "4.19.0", + "publish": "true" } } diff --git a/package-lock.json b/package-lock.json index 5cab4036cb892..0b2b8f4f442c8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -139,7 +139,7 @@ "npm-package-arg": "^11.0.1", "npm-pick-manifest": "^9.0.0", "npm-profile": "^9.0.0", - "npm-registry-fetch": "^16.0.0", + "npm-registry-fetch": "^16.1.0", "npm-user-validate": "^2.0.0", "npmlog": "^7.0.1", "p-map": "^4.0.0", @@ -10927,9 +10927,9 @@ } }, "node_modules/npm-registry-fetch": { - "version": "16.0.0", - "resolved": "https://registry.npmjs.org/npm-registry-fetch/-/npm-registry-fetch-16.0.0.tgz", - "integrity": "sha512-JFCpAPUpvpwfSydv99u85yhP68rNIxSFmDpNbNnRWKSe3gpjHnWL8v320gATwRzjtgmZ9Jfe37+ZPOLZPwz6BQ==", + "version": "16.1.0", + "resolved": "https://registry.npmjs.org/npm-registry-fetch/-/npm-registry-fetch-16.1.0.tgz", + "integrity": "sha512-PQCELXKt8Azvxnt5Y85GseQDJJlglTFM9L9U9gkv2y4e9s0k3GVDdOx3YoB6gm2Do0hlkzC39iCGXby+Wve1Bw==", "inBundle": true, "dependencies": { "make-fetch-happen": "^13.0.0", diff --git a/package.json b/package.json index 3246377f0cb72..e0bceed98ceb1 100644 --- a/package.json +++ b/package.json @@ -101,7 +101,7 @@ "npm-package-arg": "^11.0.1", "npm-pick-manifest": "^9.0.0", "npm-profile": "^9.0.0", - "npm-registry-fetch": "^16.0.0", + "npm-registry-fetch": "^16.1.0", "npm-user-validate": "^2.0.0", "npmlog": "^7.0.1", "p-map": "^4.0.0", From fcecf8bc15e5ca2d91b0cb69d73c4064e0ea9112 Mon Sep 17 00:00:00 2001 From: Gar Date: Fri, 6 Oct 2023 10:06:21 -0700 Subject: [PATCH 2/5] fix: delete auth from proper location on logout --- lib/commands/logout.js | 9 ++++++--- workspaces/config/lib/index.js | 32 ++++++++++++++++---------------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/lib/commands/logout.js b/lib/commands/logout.js index aea5e93652b0e..ef0f256568d2c 100644 --- a/lib/commands/logout.js +++ b/lib/commands/logout.js @@ -19,6 +19,9 @@ class Logout extends BaseCommand { const auth = getAuth(reg, this.npm.flatOptions) + const level = this.npm.config.find(`${auth.regKey}:${auth.authKey}`) + + // find the config level and only delete from there if (auth.token) { log.verbose('logout', `clearing token for ${reg}`) await npmFetch(`/-/user/token/${encodeURIComponent(auth.token)}`, { @@ -34,12 +37,12 @@ class Logout extends BaseCommand { } if (scope) { - this.npm.config.delete(regRef, 'user') + this.npm.config.delete(regRef, level) } - this.npm.config.clearCredentialsByURI(reg) + this.npm.config.clearCredentialsByURI(reg, level) - await this.npm.config.save('user') + await this.npm.config.save(level) } } module.exports = Logout diff --git a/workspaces/config/lib/index.js b/workspaces/config/lib/index.js index ad07fcdf51826..b09ecc478f64f 100644 --- a/workspaces/config/lib/index.js +++ b/workspaces/config/lib/index.js @@ -774,29 +774,29 @@ class Config { await chmod(conf.source, mode) } - clearCredentialsByURI (uri) { + clearCredentialsByURI (uri, level = 'user') { const nerfed = nerfDart(uri) const def = nerfDart(this.get('registry')) if (def === nerfed) { - this.delete(`-authtoken`, 'user') - this.delete(`_authToken`, 'user') - this.delete(`_authtoken`, 'user') - this.delete(`_auth`, 'user') - this.delete(`_password`, 'user') - this.delete(`username`, 'user') + this.delete(`-authtoken`, level) + this.delete(`_authToken`, level) + this.delete(`_authtoken`, level) + this.delete(`_auth`, level) + this.delete(`_password`, level) + this.delete(`username`, level) // de-nerf email if it's nerfed to the default registry - const email = this.get(`${nerfed}:email`, 'user') + const email = this.get(`${nerfed}:email`, level) if (email) { - this.set('email', email, 'user') + this.set('email', email, level) } } - this.delete(`${nerfed}:_authToken`, 'user') - this.delete(`${nerfed}:_auth`, 'user') - this.delete(`${nerfed}:_password`, 'user') - this.delete(`${nerfed}:username`, 'user') - this.delete(`${nerfed}:email`, 'user') - this.delete(`${nerfed}:certfile`, 'user') - this.delete(`${nerfed}:keyfile`, 'user') + this.delete(`${nerfed}:_authToken`, level) + this.delete(`${nerfed}:_auth`, level) + this.delete(`${nerfed}:_password`, level) + this.delete(`${nerfed}:username`, level) + this.delete(`${nerfed}:email`, level) + this.delete(`${nerfed}:certfile`, level) + this.delete(`${nerfed}:keyfile`, level) } setCredentialsByURI (uri, { token, username, password, email, certfile, keyfile }) { From 704013a666d4d089e5547aa1700661665b28765b Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 10 Oct 2023 11:41:41 -0700 Subject: [PATCH 3/5] fix: logout from custom registry --- lib/commands/logout.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/commands/logout.js b/lib/commands/logout.js index ef0f256568d2c..665580930639c 100644 --- a/lib/commands/logout.js +++ b/lib/commands/logout.js @@ -1,5 +1,5 @@ -const getAuth = require('npm-registry-fetch/lib/auth.js') const npmFetch = require('npm-registry-fetch') +const { getAuth } = npmFetch const log = require('../utils/log-shim') const BaseCommand = require('../base-command.js') @@ -26,6 +26,7 @@ class Logout extends BaseCommand { log.verbose('logout', `clearing token for ${reg}`) await npmFetch(`/-/user/token/${encodeURIComponent(auth.token)}`, { ...this.npm.flatOptions, + registry: reg, method: 'DELETE', ignoreBody: true, }) From 36ead14d54864cf004328ba24c805283a4b74863 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 10 Oct 2023 12:13:13 -0700 Subject: [PATCH 4/5] chore: rewrite logout tests to use mock registry --- mock-registry/lib/index.js | 5 ++ test/lib/commands/logout.js | 169 ++++++++++++------------------------ 2 files changed, 62 insertions(+), 112 deletions(-) diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index 91f1de5b52e0d..8664ac56fbefb 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -252,6 +252,11 @@ class MockRegistry { .reply(200, { token }) } + logout (token) { + this.nock = this.nock.delete(this.fullPath(`/-/user/token/${encodeURIComponent(token)}`)) + .reply(200, { ok: true }) + } + // team can be a team or a username getPackages ({ user, team, packages = {}, times = 1, responseCode = 200 }) { let uri diff --git a/test/lib/commands/logout.js b/test/lib/commands/logout.js index 4ff21665f3035..f83aeef7854a9 100644 --- a/test/lib/commands/logout.js +++ b/test/lib/commands/logout.js @@ -1,170 +1,115 @@ const t = require('tap') const fs = require('fs/promises') -const npmFetch = require('npm-registry-fetch') -const mockNpm = require('../../fixtures/mock-npm') +const { load: loadMockNpm } = require('../../fixtures/mock-npm.js') +const MockRegistry = require('@npmcli/mock-registry') const { join } = require('path') -const mockLogout = async (t, { userRc = [], ...npmOpts } = {}) => { - let result = null - - const mock = await mockNpm(t, { - command: 'logout', - mocks: { - // XXX: refactor to use mock registry - 'npm-registry-fetch': Object.assign(async (url, opts) => { - result = { url, opts } - }, npmFetch), - }, - ...npmOpts, +t.test('token logout - user config', async t => { + const { npm, home, logs } = await loadMockNpm(t, { homeDir: { - '.npmrc': userRc.join('\n'), + '.npmrc': [ + '//registry.npmjs.org/:_authToken=@foo/', + 'other-config=true', + ].join('\n'), }, }) - return { - ...mock, - result: () => result, - // get only the message portion of the verbose log from the command - logMsg: () => mock.logs.verbose.find(l => l[0] === 'logout')[1], - userRc: () => fs.readFile(join(mock.home, '.npmrc'), 'utf-8').then(r => r.trim()), - } -} - -t.test('token logout', async t => { - const { logout, logMsg, result, userRc } = await mockLogout(t, { - userRc: [ - '//registry.npmjs.org/:_authToken=@foo/', - 'other-config=true', - ], - }) - - await logout.exec([]) - + const mockRegistry = new MockRegistry({ tap: t, registry: 'https://registry.npmjs.org/' }) + mockRegistry.logout('@foo/') + await npm.exec('logout', []) t.equal( - logMsg(), + logs.verbose.find(l => l[0] === 'logout')[1], 'clearing token for https://registry.npmjs.org/', 'should log message with correct registry' ) + const userRc = await fs.readFile(join(home, '.npmrc'), 'utf-8') + t.equal(userRc.trim(), 'other-config=true') +}) - t.match( - result(), - { - url: '/-/user/token/%40foo%2F', - opts: { - registry: 'https://registry.npmjs.org/', - scope: '', - '//registry.npmjs.org/:_authToken': '@foo/', - method: 'DELETE', - ignoreBody: true, - }, +t.test('token scoped logout - user config', async t => { + const { npm, home, logs } = await loadMockNpm(t, { + config: { + scope: '@myscope', }, - 'should call npm-registry-fetch with expected values' - ) - - t.equal(await userRc(), 'other-config=true') -}) + homeDir: { + '.npmrc': [ + '//diff-registry.npmjs.com/:_authToken=@bar/', + '//registry.npmjs.org/:_authToken=@foo/', + '@myscope:registry=https://diff-registry.npmjs.com/', -t.test('token scoped logout', async t => { - const { logout, logMsg, result, userRc } = await mockLogout(t, { - config: { scope: '@myscope' }, - userRc: [ - '//diff-registry.npmjs.com/:_authToken=@bar/', - '//registry.npmjs.org/:_authToken=@foo/', - '@myscope:registry=https://diff-registry.npmjs.com/', - ], + ].join('\n'), + }, }) - await logout.exec([]) - + const mockRegistry = new MockRegistry({ tap: t, registry: 'https://diff-registry.npmjs.com/' }) + mockRegistry.logout('@bar/') + await npm.exec('logout', []) t.equal( - logMsg(), + logs.verbose.find(l => l[0] === 'logout')[1], 'clearing token for https://diff-registry.npmjs.com/', 'should log message with correct registry' ) - t.match( - result(), - { - url: '/-/user/token/%40bar%2F', - opts: { - registry: 'https://registry.npmjs.org/', - '@myscope:registry': 'https://diff-registry.npmjs.com/', - scope: '@myscope', - '//registry.npmjs.org/:_authToken': '@foo/', // <- removed by npm-registry-fetch - '//diff-registry.npmjs.com/:_authToken': '@bar/', - method: 'DELETE', - ignoreBody: true, - }, - }, - 'should call npm-registry-fetch with expected values' - ) - - t.equal(await userRc(), '//registry.npmjs.org/:_authToken=@foo/') + const userRc = await fs.readFile(join(home, '.npmrc'), 'utf-8') + t.equal(userRc.trim(), '//registry.npmjs.org/:_authToken=@foo/') }) -t.test('user/pass logout', async t => { - const { logout, logMsg, userRc } = await mockLogout(t, { - userRc: [ +t.test('user/pass logout - user config', async t => { + const { npm, home, logs } = await loadMockNpm(t, { + homeDir: { + '.npmrc': [ '//registry.npmjs.org/:username=foo', '//registry.npmjs.org/:_password=bar', 'other-config=true', - ], + ].join('\n'), + }, }) - await logout.exec([]) - + await npm.exec('logout', []) t.equal( - logMsg(), + logs.verbose.find(l => l[0] === 'logout')[1], 'clearing user credentials for https://registry.npmjs.org/', 'should log message with correct registry' ) - t.equal(await userRc(), 'other-config=true') + const userRc = await fs.readFile(join(home, '.npmrc'), 'utf-8') + t.equal(userRc.trim(), 'other-config=true') }) t.test('missing credentials', async t => { - const { logout } = await mockLogout(t) + const { npm, logs } = await loadMockNpm(t) await t.rejects( - logout.exec([]), + npm.exec('logout', []), { code: 'ENEEDAUTH', message: /not logged in to https:\/\/registry.npmjs.org\/, so can't log out!/, }, - 'should throw with expected error code' + 'should reject with expected error code' ) }) t.test('ignore invalid scoped registry config', async t => { - const { logout, logMsg, result, userRc } = await mockLogout(t, { + const { npm, home, logs } = await loadMockNpm(t, { config: { scope: '@myscope' }, - userRc: [ + homeDir: { + '.npmrc': [ '//registry.npmjs.org/:_authToken=@foo/', 'other-config=true', - ], + + ].join('\n'), + }, }) - await logout.exec([]) + const mockRegistry = new MockRegistry({ tap: t, registry: 'https://registry.npmjs.org/' }) + mockRegistry.logout('@foo/') + await npm.exec('logout', []) t.equal( - logMsg(), + logs.verbose.find(l => l[0] === 'logout')[1], 'clearing token for https://registry.npmjs.org/', 'should log message with correct registry' ) - - t.match( - result(), - { - url: '/-/user/token/%40foo%2F', - opts: { - '//registry.npmjs.org/:_authToken': '@foo/', - registry: 'https://registry.npmjs.org/', - method: 'DELETE', - ignoreBody: true, - }, - }, - 'should call npm-registry-fetch with expected values' - ) - - t.equal(await userRc(), 'other-config=true') + const userRc = await fs.readFile(join(home, '.npmrc'), 'utf-8') + t.equal(userRc.trim(), 'other-config=true') }) From 77ec6a58988b0eea3f0f0c85f56a4d7038c5a24a Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 10 Oct 2023 12:20:54 -0700 Subject: [PATCH 5/5] chore: add tests for logout with project config --- test/lib/commands/logout.js | 51 ++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/test/lib/commands/logout.js b/test/lib/commands/logout.js index f83aeef7854a9..881003729ab4a 100644 --- a/test/lib/commands/logout.js +++ b/test/lib/commands/logout.js @@ -58,9 +58,9 @@ t.test('user/pass logout - user config', async t => { const { npm, home, logs } = await loadMockNpm(t, { homeDir: { '.npmrc': [ - '//registry.npmjs.org/:username=foo', - '//registry.npmjs.org/:_password=bar', - 'other-config=true', + '//registry.npmjs.org/:username=foo', + '//registry.npmjs.org/:_password=bar', + 'other-config=true', ].join('\n'), }, }) @@ -77,7 +77,7 @@ t.test('user/pass logout - user config', async t => { }) t.test('missing credentials', async t => { - const { npm, logs } = await loadMockNpm(t) + const { npm } = await loadMockNpm(t) await t.rejects( npm.exec('logout', []), @@ -94,8 +94,8 @@ t.test('ignore invalid scoped registry config', async t => { config: { scope: '@myscope' }, homeDir: { '.npmrc': [ - '//registry.npmjs.org/:_authToken=@foo/', - 'other-config=true', + '//registry.npmjs.org/:_authToken=@foo/', + 'other-config=true', ].join('\n'), }, @@ -113,3 +113,42 @@ t.test('ignore invalid scoped registry config', async t => { const userRc = await fs.readFile(join(home, '.npmrc'), 'utf-8') t.equal(userRc.trim(), 'other-config=true') }) + +t.test('token logout - project config', async t => { + const { npm, home, logs, prefix } = await loadMockNpm(t, { + homeDir: { + '.npmrc': [ + '//registry.npmjs.org/:_authToken=@foo/', + 'other-config=true', + ].join('\n'), + }, + prefixDir: { + '.npmrc': [ + '//registry.npmjs.org/:_authToken=@bar/', + 'other-config=true', + ].join('\n'), + }, + }) + + const mockRegistry = new MockRegistry({ tap: t, registry: 'https://registry.npmjs.org/' }) + mockRegistry.logout('@bar/') + await npm.exec('logout', []) + + t.equal( + logs.verbose.find(l => l[0] === 'logout')[1], + 'clearing token for https://registry.npmjs.org/', + 'should log message with correct registry' + ) + const userRc = await fs.readFile(join(home, '.npmrc'), 'utf-8') + t.equal(userRc.trim(), [ + '//registry.npmjs.org/:_authToken=@foo/', + 'other-config=true', + ].join('\n'), 'leaves user config alone') + t.equal( + logs.verbose.find(l => l[0] === 'logout')[1], + 'clearing token for https://registry.npmjs.org/', + 'should log message with correct registry' + ) + const projectRc = await fs.readFile(join(prefix, '.npmrc'), 'utf-8') + t.equal(projectRc.trim(), 'other-config=true', 'removes project config') +})