From edcf81df3e9ec8c4987891071ae4606da4ccb872 Mon Sep 17 00:00:00 2001 From: Reggi Date: Tue, 15 Apr 2025 17:16:46 +0000 Subject: [PATCH 01/10] fixes enable and disable 2fa commands --- lib/commands/profile.js | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/commands/profile.js b/lib/commands/profile.js index 965fcbcb8ce29..a2e1550e01793 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -222,6 +222,8 @@ class Profile extends BaseCommand { } async enable2fa (args) { + const conf = { ...this.npm.flatOptions } + if (args.length > 1) { throw new Error('npm profile enable-2fa [auth-and-writes|auth-only]') } @@ -244,9 +246,16 @@ class Profile extends BaseCommand { ) } + const existingInfo = await get(conf) + + if (!existingInfo.tfa.pending && existingInfo.tfa.mode === mode) { + output.standard('Two factor authentication is already enabled and set to ' + mode) + return + } + const info = { tfa: { - mode: mode, + mode, }, } @@ -299,22 +308,15 @@ class Profile extends BaseCommand { log.info('profile', 'Determine if tfa is pending') const userInfo = await get({ ...this.npm.flatOptions }) - const conf = { ...this.npm.flatOptions } if (userInfo && userInfo.tfa && userInfo.tfa.pending) { log.info('profile', 'Resetting two-factor authentication') - await set({ tfa: { password, mode: 'disable' } }, conf) - } else if (userInfo && userInfo.tfa) { - if (!conf.otp) { - conf.otp = await readUserInfo.otp( - 'Enter one-time password: ' - ) - } + await otplease(this.npm, conf, c => set(({ tfa: { password, mode: 'disable' } }, conf))) } log.info('profile', 'Setting two-factor authentication to ' + mode) - const challenge = await set(info, conf) + const challenge = await otplease(this.npm, conf, c => set(info, c)) - if (challenge.tfa === null) { + if (challenge.tfa && challenge.tfa.mode) { output.standard('Two factor authentication mode changed to: ' + mode) return } @@ -358,8 +360,8 @@ class Profile extends BaseCommand { } async disable2fa () { - const conf = { ...this.npm.flatOptions } - const info = await get(conf) + const opts = { ...this.npm.flatOptions } + const info = await get(opts) if (!info.tfa || info.tfa.pending) { output.standard('Two factor authentication not enabled.') @@ -368,14 +370,8 @@ class Profile extends BaseCommand { const password = await readUserInfo.password() - if (!conf.otp) { - const msg = 'Enter one-time password: ' - conf.otp = await readUserInfo.otp(msg) - } - log.info('profile', 'disabling tfa') - - await set({ tfa: { password: password, mode: 'disable' } }, conf) + await otplease(this.npm, opts, o => set({ tfa: { password: password, mode: 'disable' } }, o)) if (this.npm.config.get('json')) { output.buffer({ tfa: false }) From f80bd09662de1419764ed52b26acfa270b63cd7f Mon Sep 17 00:00:00 2001 From: Reggi Date: Tue, 15 Apr 2025 17:19:37 +0000 Subject: [PATCH 02/10] fix lint --- lib/commands/profile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/commands/profile.js b/lib/commands/profile.js index a2e1550e01793..30c503d9ad7a6 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -310,7 +310,7 @@ class Profile extends BaseCommand { if (userInfo && userInfo.tfa && userInfo.tfa.pending) { log.info('profile', 'Resetting two-factor authentication') - await otplease(this.npm, conf, c => set(({ tfa: { password, mode: 'disable' } }, conf))) + await otplease(this.npm, conf, c => set(({ tfa: { password, mode: 'disable' } }, c))) } log.info('profile', 'Setting two-factor authentication to ' + mode) From eb095656239a6d543b70361aaba2b1939ff54c52 Mon Sep 17 00:00:00 2001 From: Reggi Date: Tue, 15 Apr 2025 18:16:14 +0000 Subject: [PATCH 03/10] update --- lib/commands/profile.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/commands/profile.js b/lib/commands/profile.js index 30c503d9ad7a6..199bb12b345dc 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -246,9 +246,10 @@ class Profile extends BaseCommand { ) } - const existingInfo = await get(conf) + log.info('profile', 'Determine if tfa is pending') + const userInfo = await get(conf) - if (!existingInfo.tfa.pending && existingInfo.tfa.mode === mode) { + if (!userInfo.tfa.pending && userInfo.tfa.mode === mode) { output.standard('Two factor authentication is already enabled and set to ' + mode) return } @@ -305,12 +306,9 @@ class Profile extends BaseCommand { const password = await readUserInfo.password() info.tfa.password = password - log.info('profile', 'Determine if tfa is pending') - const userInfo = await get({ ...this.npm.flatOptions }) - if (userInfo && userInfo.tfa && userInfo.tfa.pending) { log.info('profile', 'Resetting two-factor authentication') - await otplease(this.npm, conf, c => set(({ tfa: { password, mode: 'disable' } }, c))) + await set({ tfa: { password, mode: 'disable' } }, conf) } log.info('profile', 'Setting two-factor authentication to ' + mode) From b8f25edace834582379099e3f355bf685550b5ba Mon Sep 17 00:00:00 2001 From: reggi Date: Thu, 17 Apr 2025 14:48:04 -0400 Subject: [PATCH 04/10] fixes tests --- lib/commands/profile.js | 6 +- lib/utils/auth.js | 1 - test/lib/commands/profile.js | 160 +++++++++++++++++++++++------------ 3 files changed, 112 insertions(+), 55 deletions(-) diff --git a/lib/commands/profile.js b/lib/commands/profile.js index 199bb12b345dc..f17ddc5900085 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -249,7 +249,7 @@ class Profile extends BaseCommand { log.info('profile', 'Determine if tfa is pending') const userInfo = await get(conf) - if (!userInfo.tfa.pending && userInfo.tfa.mode === mode) { + if (!userInfo?.tfa?.pending && userInfo?.tfa?.mode === mode) { output.standard('Two factor authentication is already enabled and set to ' + mode) return } @@ -312,7 +312,9 @@ class Profile extends BaseCommand { } log.info('profile', 'Setting two-factor authentication to ' + mode) - const challenge = await otplease(this.npm, conf, c => set(info, c)) + const challenge = await otplease(this.npm, conf, async c => { + return await set(info, c) + }) if (challenge.tfa && challenge.tfa.mode) { output.standard('Two factor authentication mode changed to: ' + mode) diff --git a/lib/utils/auth.js b/lib/utils/auth.js index 747271169124b..f8e7bd85d8353 100644 --- a/lib/utils/auth.js +++ b/lib/utils/auth.js @@ -10,7 +10,6 @@ const otplease = async (npm, opts, fn) => { if (!process.stdin.isTTY || !process.stdout.isTTY) { throw err } - // web otp if (err.code === 'EOTP' && err.body?.authUrl && err.body?.doneUrl) { const { token: otp } = await webAuthOpener( diff --git a/test/lib/commands/profile.js b/test/lib/commands/profile.js index 8bbffd1675d07..2204f2e9df1fb 100644 --- a/test/lib/commands/profile.js +++ b/test/lib/commands/profile.js @@ -1,7 +1,16 @@ +const { inspect } = require('node:util') const t = require('tap') const mockNpm = require('../../fixtures/mock-npm') +const tmock = require('../../fixtures/tmock') -const mockProfile = async (t, { npmProfile, readUserInfo, qrcode, config, ...opts } = {}) => { +const mockProfile = async (t, { + npmProfile, readUserInfo, qrcode, config, isTTY, ...opts } = {}) => { + const mockReadUserInfo = { + '{LIB}/utils/read-user-info.js': readUserInfo || { + async password () {}, + async otp () {}, + }, + } const mocks = { 'npm-profile': npmProfile || { async get () {}, @@ -9,10 +18,8 @@ const mockProfile = async (t, { npmProfile, readUserInfo, qrcode, config, ...opt async createToken () {}, }, 'qrcode-terminal': qrcode || { generate: (url, cb) => cb() }, - '{LIB}/utils/read-user-info.js': readUserInfo || { - async password () {}, - async otp () {}, - }, + ...mockReadUserInfo, + '{LIB}/utils/auth.js': tmock(t, '{LIB}/utils/auth.js', mockReadUserInfo), } const mock = await mockNpm(t, { @@ -22,6 +29,10 @@ const mockProfile = async (t, { npmProfile, readUserInfo, qrcode, config, ...opt color: false, ...config, }, + globals: { + 'process.stdin.isTTY': isTTY, + 'process.stdout.isTTY': isTTY, + }, mocks: { ...mocks, ...opts.mocks, @@ -516,6 +527,12 @@ t.test('enable-2fa', async t => { t.match(pass, 'bar', 'should use password for basic auth') return {} }, + async get () { + return { + userProfile, + tfa: null, + } + }, } const { npm, profile } = await mockProfile(t, { @@ -543,6 +560,12 @@ t.test('enable-2fa', async t => { async createToken () { return {} }, + async get () { + return { + ...userProfile, + tfa: null, + } + }, } const { npm, profile } = await mockProfile(t, { @@ -577,7 +600,9 @@ t.test('enable-2fa', async t => { }) t.test('from basic auth, asks for otp', async t => { - t.plan(9) + t.plan(10) + + let setCallCount = 0 const npmProfile = { async createToken (pass) { @@ -588,18 +613,36 @@ t.test('enable-2fa', async t => { return userProfile }, async set (newProfile) { - t.match( - newProfile, - { + setCallCount++ + if (setCallCount === 1) { + t.match( + newProfile, + { + tfa: { + mode: 'auth-only', + }, + }, + 'should set tfa mode on first call' + ) + const err = new Error('One-time password required') + err.code = 'EOTP' + throw err + } else if (setCallCount === 2) { + t.match( + newProfile, + { + tfa: { + mode: 'auth-only', + }, + }, + 'should set tfa mode' + ) + return { + ...userProfile, tfa: { mode: 'auth-only', }, - }, - 'should set tfa mode' - ) - return { - ...userProfile, - tfa: null, + } } }, } @@ -612,7 +655,7 @@ t.test('enable-2fa', async t => { async otp (label) { t.equal( label, - 'Enter one-time password: ', + 'This operation requires a one-time password.\nEnter OTP:', 'should ask for otp confirmation' ) return '123456' @@ -620,6 +663,7 @@ t.test('enable-2fa', async t => { } const { npm, profile, result } = await mockProfile(t, { + isTTY: true, npmProfile, readUserInfo, }) @@ -817,12 +861,12 @@ t.test('enable-2fa', async t => { t.equal( result(), - 'Two factor authentication mode changed to: auth-and-writes', + 'Two factor authentication is already enabled and set to auth-and-writes', 'should output success msg' ) }) - t.test('missing tfa from user profile', async t => { + t.test('errors when tfa is return null (not otpauth URL) and tfa is not setup already (with auth-only)', async t => { const npmProfile = { async get () { return { @@ -847,7 +891,7 @@ t.test('enable-2fa', async t => { }, } - const { npm, profile, result } = await mockProfile(t, { + const { npm, profile } = await mockProfile(t, { npmProfile, readUserInfo, }) @@ -856,16 +900,15 @@ t.test('enable-2fa', async t => { return { token: 'token' } } - await profile.exec(['enable-2fa', 'auth-only']) - - t.equal( - result(), - 'Two factor authentication mode changed to: auth-only', - 'should output success msg' - ) + await t.rejects(async () => { + await profile.exec(['enable-2fa', 'auth-only']) + }, new Error( + 'Unknown error enabling two-factor authentication. Expected otpauth URL' + + ', got: ' + inspect(null) + )) }) - t.test('defaults to auth-and-writes permission if no mode specified', async t => { + t.test('errors when tfa is return null (not otpauth URL) and tfa is not setup already', async t => { const npmProfile = { async get () { return { @@ -890,7 +933,7 @@ t.test('enable-2fa', async t => { }, } - const { npm, profile, result } = await mockProfile(t, { + const { npm, profile } = await mockProfile(t, { npmProfile, readUserInfo, }) @@ -899,12 +942,12 @@ t.test('enable-2fa', async t => { return { token: 'token' } } - await profile.exec(['enable-2fa']) - t.equal( - result(), - 'Two factor authentication mode changed to: auth-and-writes', - 'should enable 2fa with auth-and-writes permission' - ) + await t.rejects(async () => { + await profile.exec(['enable-2fa']) + }, new Error( + 'Unknown error enabling two-factor authentication. Expected otpauth URL' + + ', got: ' + inspect(null) + )) }) }) @@ -929,23 +972,33 @@ t.test('disable-2fa', async t => { }) t.test('requests otp', async t => { - const npmProfile = t => ({ - async get () { - return userProfile - }, - async set (newProfile) { - t.same( - newProfile, - { - tfa: { - password: 'password1234', - mode: 'disable', - }, - }, - 'should send the new info for setting in profile' - ) - }, - }) + const OTP_ERROR = Object.assign(new Error('One-time password required'), { code: 'EOTP' }) + + const npmProfile = (t) => { + let setCallCount = 0 + return { + async get () { + return userProfile + }, + async set (newProfile) { + setCallCount++ + if (setCallCount === 1) { + throw OTP_ERROR + } else if (setCallCount === 2) { + t.same( + newProfile, + { + tfa: { + password: 'password1234', + mode: 'disable', + }, + }, + 'should send the new info for setting in profile' + ) + } + }, + } + } const readUserInfo = t => ({ async password () { @@ -955,7 +1008,7 @@ t.test('disable-2fa', async t => { async otp (label) { t.equal( label, - 'Enter one-time password: ', + 'This operation requires a one-time password.\nEnter OTP:', 'should ask for otp confirmation' ) return '1234' @@ -968,6 +1021,7 @@ t.test('disable-2fa', async t => { const { profile, result } = await mockProfile(t, { npmProfile: npmProfile(t), readUserInfo: readUserInfo(t), + isTTY: true, }) await profile.exec(['disable-2fa']) @@ -983,6 +1037,7 @@ t.test('disable-2fa', async t => { npmProfile: npmProfile(t), readUserInfo: readUserInfo(t), config, + isTTY: true, }) await profile.exec(['disable-2fa']) @@ -999,6 +1054,7 @@ t.test('disable-2fa', async t => { npmProfile: npmProfile(t), readUserInfo: readUserInfo(t), config, + isTTY: true, }) await profile.exec(['disable-2fa']) From 4961c3698779e8bb1cbbcc92257d692a4e24343d Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 23 Apr 2025 17:32:29 -0400 Subject: [PATCH 05/10] collapes callback --- lib/commands/profile.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/commands/profile.js b/lib/commands/profile.js index f17ddc5900085..dfc0c702faa4b 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -312,9 +312,7 @@ class Profile extends BaseCommand { } log.info('profile', 'Setting two-factor authentication to ' + mode) - const challenge = await otplease(this.npm, conf, async c => { - return await set(info, c) - }) + const challenge = await otplease(this.npm, conf, c => set(info, c)) if (challenge.tfa && challenge.tfa.mode) { output.standard('Two factor authentication mode changed to: ' + mode) From f2d9371b4c63673e8bed99ec1e9a9fa8eee7389e Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 25 Apr 2025 11:20:11 -0400 Subject: [PATCH 06/10] add otplease for access commands --- lib/commands/access.js | 8 ++++++-- node_modules/npm-registry-fetch/lib/index.js | 15 +++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/commands/access.js b/lib/commands/access.js index 547fa7af01577..d0faf394f0966 100644 --- a/lib/commands/access.js +++ b/lib/commands/access.js @@ -116,11 +116,15 @@ class Access extends BaseCommand { } async #grant (permissions, scope, pkg) { - await libnpmaccess.setPermissions(scope, pkg, permissions, this.npm.flatOptions) + await otplease(this.npm, this.npm.flatOptions, async (opts) => { + await libnpmaccess.setPermissions(scope, pkg, permissions, opts) + }) } async #revoke (scope, pkg) { - await libnpmaccess.removePermissions(scope, pkg, this.npm.flatOptions) + await otplease(this.npm, this.npm.flatOptions, async (opts) => { + await libnpmaccess.removePermissions(scope, pkg, opts) + }) } async #listPackages (owner, pkg) { diff --git a/node_modules/npm-registry-fetch/lib/index.js b/node_modules/npm-registry-fetch/lib/index.js index 898c8125bfe0e..99c0f7bb68222 100644 --- a/node_modules/npm-registry-fetch/lib/index.js +++ b/node_modules/npm-registry-fetch/lib/index.js @@ -123,14 +123,16 @@ function regFetch (uri, /* istanbul ignore next */ opts_ = {}) { noProxy: opts.noProxy, proxy: opts.httpsProxy || opts.proxy, retry: opts.retry ? opts.retry : { - retries: opts.fetchRetries, - factor: opts.fetchRetryFactor, - minTimeout: opts.fetchRetryMintimeout, - maxTimeout: opts.fetchRetryMaxtimeout, + retries: opts.fetchRetries, + factor: opts.fetchRetryFactor, + minTimeout: opts.fetchRetryMintimeout, + maxTimeout: opts.fetchRetryMaxtimeout, }, strictSSL: opts.strictSSL, timeout: opts.timeout || 30 * 1000, - }).then(res => checkResponse({ + }).then(res => { + console.log('Response Headers:', res.headers.raw()); + return checkResponse({ method, uri, res, @@ -138,7 +140,8 @@ function regFetch (uri, /* istanbul ignore next */ opts_ = {}) { startTime, auth, opts, - })) + }); + }) if (typeof opts.otpPrompt === 'function') { return p.catch(async er => { From f236b944c29edd3b0e1524bda0883a0735d5c74e Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 25 Apr 2025 13:24:06 -0400 Subject: [PATCH 07/10] undo log headers --- node_modules/npm-registry-fetch/lib/index.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/node_modules/npm-registry-fetch/lib/index.js b/node_modules/npm-registry-fetch/lib/index.js index 99c0f7bb68222..898c8125bfe0e 100644 --- a/node_modules/npm-registry-fetch/lib/index.js +++ b/node_modules/npm-registry-fetch/lib/index.js @@ -123,16 +123,14 @@ function regFetch (uri, /* istanbul ignore next */ opts_ = {}) { noProxy: opts.noProxy, proxy: opts.httpsProxy || opts.proxy, retry: opts.retry ? opts.retry : { - retries: opts.fetchRetries, - factor: opts.fetchRetryFactor, - minTimeout: opts.fetchRetryMintimeout, - maxTimeout: opts.fetchRetryMaxtimeout, + retries: opts.fetchRetries, + factor: opts.fetchRetryFactor, + minTimeout: opts.fetchRetryMintimeout, + maxTimeout: opts.fetchRetryMaxtimeout, }, strictSSL: opts.strictSSL, timeout: opts.timeout || 30 * 1000, - }).then(res => { - console.log('Response Headers:', res.headers.raw()); - return checkResponse({ + }).then(res => checkResponse({ method, uri, res, @@ -140,8 +138,7 @@ function regFetch (uri, /* istanbul ignore next */ opts_ = {}) { startTime, auth, opts, - }); - }) + })) if (typeof opts.otpPrompt === 'function') { return p.catch(async er => { From a0d1cc3be1766fec3a46a85aadcb8cc2f29b0b5a Mon Sep 17 00:00:00 2001 From: Reggi Date: Fri, 25 Apr 2025 13:58:23 -0400 Subject: [PATCH 08/10] Update lib/utils/auth.js --- lib/utils/auth.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/utils/auth.js b/lib/utils/auth.js index f8e7bd85d8353..747271169124b 100644 --- a/lib/utils/auth.js +++ b/lib/utils/auth.js @@ -10,6 +10,7 @@ const otplease = async (npm, opts, fn) => { if (!process.stdin.isTTY || !process.stdout.isTTY) { throw err } + // web otp if (err.code === 'EOTP' && err.body?.authUrl && err.body?.doneUrl) { const { token: otp } = await webAuthOpener( From f7ccfc9ffec51dd3e763fae71d139cf19e9efa0b Mon Sep 17 00:00:00 2001 From: Reggi Date: Tue, 29 Apr 2025 14:15:40 -0400 Subject: [PATCH 09/10] Update lib/commands/profile.js --- lib/commands/profile.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/commands/profile.js b/lib/commands/profile.js index dfc0c702faa4b..dab46fcd50ed1 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -246,7 +246,6 @@ class Profile extends BaseCommand { ) } - log.info('profile', 'Determine if tfa is pending') const userInfo = await get(conf) if (!userInfo?.tfa?.pending && userInfo?.tfa?.mode === mode) { From fa48bafde1d799e14c241f5a9c1bb9f13e76a23f Mon Sep 17 00:00:00 2001 From: Reggi Date: Tue, 6 May 2025 15:12:53 -0400 Subject: [PATCH 10/10] Update lib/commands/profile.js Co-authored-by: Gar --- lib/commands/profile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/commands/profile.js b/lib/commands/profile.js index dab46fcd50ed1..2e11f93788f99 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -311,7 +311,7 @@ class Profile extends BaseCommand { } log.info('profile', 'Setting two-factor authentication to ' + mode) - const challenge = await otplease(this.npm, conf, c => set(info, c)) + const challenge = await otplease(this.npm, conf, o => set(info, o)) if (challenge.tfa && challenge.tfa.mode) { output.standard('Two factor authentication mode changed to: ' + mode)