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/lib/commands/profile.js b/lib/commands/profile.js index 965fcbcb8ce29..dab46fcd50ed1 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 userInfo = await get(conf) + + if (!userInfo?.tfa?.pending && userInfo?.tfa?.mode === mode) { + output.standard('Two factor authentication is already enabled and set to ' + mode) + return + } + const info = { tfa: { - mode: mode, + mode, }, } @@ -296,25 +305,15 @@ 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 }) - - 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: ' - ) - } } 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 +357,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 +367,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 }) 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'])