From 261ea193c96aaa73ce5630e21c6a31de9f19ef5b Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 2 May 2024 09:15:04 -0700 Subject: [PATCH] fix: run input.start around help and openining urls This also refactors auth and prompt related files to use promises and new npm-profile behavior --- lib/commands/access.js | 2 +- lib/commands/deprecate.js | 2 +- lib/commands/dist-tag.js | 2 +- lib/commands/fund.js | 3 +- lib/commands/help.js | 10 +- lib/commands/hook.js | 2 +- lib/commands/org.js | 2 +- lib/commands/owner.js | 2 +- lib/commands/profile.js | 24 +- lib/commands/publish.js | 2 +- lib/commands/team.js | 2 +- lib/commands/token.js | 12 +- lib/commands/unpublish.js | 2 +- lib/package-url-cmd.js | 2 +- lib/utils/auth.js | 63 ++-- lib/utils/open-url-prompt.js | 66 ---- lib/utils/open-url.js | 108 ++++-- lib/utils/otplease.js | 46 --- lib/utils/read-user-info.js | 12 +- lib/utils/web-auth.js | 20 - mock-registry/lib/index.js | 4 +- .../lib/utils/open-url-prompt.js.test.cjs | 22 -- .../test/lib/utils/open-url.js.test.cjs | 32 +- test/lib/commands/adduser.js | 4 +- test/lib/commands/bugs.js | 2 +- test/lib/commands/docs.js | 2 +- test/lib/commands/repo.js | 8 +- test/lib/utils/{otplease.js => auth.js} | 31 +- test/lib/utils/open-url-prompt.js | 156 -------- test/lib/utils/open-url.js | 352 +++++++++++++----- test/lib/utils/web-auth.js | 33 -- 31 files changed, 459 insertions(+), 571 deletions(-) delete mode 100644 lib/utils/open-url-prompt.js delete mode 100644 lib/utils/otplease.js delete mode 100644 lib/utils/web-auth.js delete mode 100644 tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs rename test/lib/utils/{otplease.js => auth.js} (87%) delete mode 100644 test/lib/utils/open-url-prompt.js delete mode 100644 test/lib/utils/web-auth.js diff --git a/lib/commands/access.js b/lib/commands/access.js index d35699e839109..dad0c405f713b 100644 --- a/lib/commands/access.js +++ b/lib/commands/access.js @@ -3,7 +3,7 @@ const npa = require('npm-package-arg') const { output } = require('proc-log') const pkgJson = require('@npmcli/package-json') const localeCompare = require('@isaacs/string-locale-compare')('en') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const getIdentity = require('../utils/get-identity.js') const BaseCommand = require('../base-cmd.js') diff --git a/lib/commands/deprecate.js b/lib/commands/deprecate.js index 58856538fe23f..38925804f1137 100644 --- a/lib/commands/deprecate.js +++ b/lib/commands/deprecate.js @@ -1,5 +1,5 @@ const fetch = require('npm-registry-fetch') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const npa = require('npm-package-arg') const semver = require('semver') const getIdentity = require('../utils/get-identity.js') diff --git a/lib/commands/dist-tag.js b/lib/commands/dist-tag.js index e13f9ecf59c7f..663f0eb44a26a 100644 --- a/lib/commands/dist-tag.js +++ b/lib/commands/dist-tag.js @@ -2,7 +2,7 @@ const npa = require('npm-package-arg') const regFetch = require('npm-registry-fetch') const semver = require('semver') const { log, output } = require('proc-log') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const pkgJson = require('@npmcli/package-json') const BaseCommand = require('../base-cmd.js') diff --git a/lib/commands/fund.js b/lib/commands/fund.js index 8bcd184e70968..8bb4c304b379b 100644 --- a/lib/commands/fund.js +++ b/lib/commands/fund.js @@ -5,8 +5,7 @@ const { output } = require('proc-log') const npa = require('npm-package-arg') const { depth } = require('treeverse') const { readTree: getFundingInfo, normalizeFunding, isValidFunding } = require('libnpmfund') - -const openUrl = require('../utils/open-url.js') +const { openUrl } = require('../utils/open-url.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') const getPrintableName = ({ name, version }) => { diff --git a/lib/commands/help.js b/lib/commands/help.js index fb3fe664e017d..a2307bf1d8a2c 100644 --- a/lib/commands/help.js +++ b/lib/commands/help.js @@ -1,8 +1,8 @@ const spawn = require('@npmcli/promise-spawn') const path = require('path') -const openUrl = require('../utils/open-url.js') +const { openUrl } = require('../utils/open-url.js') const { glob } = require('glob') -const { output } = require('proc-log') +const { output, input } = require('proc-log') const localeCompare = require('@isaacs/string-locale-compare')('en') const { deref } = require('../utils/cmd-list.js') const BaseCommand = require('../base-cmd.js') @@ -95,13 +95,15 @@ class Help extends BaseCommand { args = ['emacsclient', ['-e', `(woman-find-file '${man}')`]] } - return spawn(...args, { stdio: 'inherit' }).catch(err => { + try { + await input.start(() => spawn(...args, { stdio: 'inherit' })) + } catch (err) { if (err.code) { throw new Error(`help process exited with code: ${err.code}`) } else { throw err } - }) + } } // Returns the path to the html version of the man page diff --git a/lib/commands/hook.js b/lib/commands/hook.js index 3b91ff539081a..43da92f3f9e6c 100644 --- a/lib/commands/hook.js +++ b/lib/commands/hook.js @@ -1,5 +1,5 @@ const hookApi = require('libnpmhook') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const relativeDate = require('tiny-relative-date') const { output } = require('proc-log') const BaseCommand = require('../base-cmd.js') diff --git a/lib/commands/org.js b/lib/commands/org.js index af67547a643db..caebadbc545e7 100644 --- a/lib/commands/org.js +++ b/lib/commands/org.js @@ -1,5 +1,5 @@ const liborg = require('libnpmorg') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const BaseCommand = require('../base-cmd.js') const { output } = require('proc-log') diff --git a/lib/commands/owner.js b/lib/commands/owner.js index 188065583198d..0f12cf9293c30 100644 --- a/lib/commands/owner.js +++ b/lib/commands/owner.js @@ -2,7 +2,7 @@ const npa = require('npm-package-arg') const npmFetch = require('npm-registry-fetch') const pacote = require('pacote') const { log, output } = require('proc-log') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const pkgJson = require('@npmcli/package-json') const BaseCommand = require('../base-cmd.js') const { redact } = require('@npmcli/redact') diff --git a/lib/commands/profile.js b/lib/commands/profile.js index 8eae6278549f5..adf534730de24 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -1,9 +1,9 @@ const { inspect } = require('util') const { URL } = require('url') const { log, output } = require('proc-log') -const npmProfile = require('npm-profile') +const { get, set, createToken } = require('npm-profile') const qrcodeTerminal = require('qrcode-terminal') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const readUserInfo = require('../utils/read-user-info.js') const BaseCommand = require('../base-cmd.js') @@ -101,7 +101,7 @@ class Profile extends BaseCommand { async get (args) { const tfa = 'two-factor auth' - const info = await npmProfile.get({ ...this.npm.flatOptions }) + const info = await get({ ...this.npm.flatOptions }) if (!info.cidr_whitelist) { delete info.cidr_whitelist @@ -199,7 +199,7 @@ class Profile extends BaseCommand { } // FIXME: Work around to not clear everything other than what we're setting - const user = await npmProfile.get(conf) + const user = await get(conf) const newUser = {} for (const key of writableProfileKeys) { @@ -208,7 +208,7 @@ class Profile extends BaseCommand { newUser[prop] = value - const result = await otplease(this.npm, conf, c => npmProfile.set(newUser, c)) + const result = await otplease(this.npm, conf, c => set(newUser, c)) if (this.npm.config.get('json')) { output.standard(JSON.stringify({ [prop]: result[prop] }, null, 2)) @@ -273,7 +273,7 @@ class Profile extends BaseCommand { if (auth.basic) { log.info('profile', 'Updating authentication to bearer token') - const result = await npmProfile.createToken( + const result = await createToken( auth.basic.password, false, [], { ...this.npm.flatOptions } ) @@ -297,12 +297,12 @@ class Profile extends BaseCommand { info.tfa.password = password log.info('profile', 'Determine if tfa is pending') - const userInfo = await npmProfile.get({ ...this.npm.flatOptions }) + 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 npmProfile.set({ tfa: { password, mode: 'disable' } }, conf) + await set({ tfa: { password, mode: 'disable' } }, conf) } else if (userInfo && userInfo.tfa) { if (!conf.otp) { conf.otp = await readUserInfo.otp( @@ -312,7 +312,7 @@ class Profile extends BaseCommand { } log.info('profile', 'Setting two-factor authentication to ' + mode) - const challenge = await npmProfile.set(info, conf) + const challenge = await set(info, conf) if (challenge.tfa === null) { output.standard('Two factor authentication mode changed to: ' + mode) @@ -341,7 +341,7 @@ class Profile extends BaseCommand { log.info('profile', 'Finalizing two-factor authentication') - const result = await npmProfile.set({ tfa: [interactiveOTP] }, conf) + const result = await set({ tfa: [interactiveOTP] }, conf) output.standard( '2FA successfully enabled. Below are your recovery codes, ' + @@ -359,7 +359,7 @@ class Profile extends BaseCommand { async disable2fa () { const conf = { ...this.npm.flatOptions } - const info = await npmProfile.get(conf) + const info = await get(conf) if (!info.tfa || info.tfa.pending) { output.standard('Two factor authentication not enabled.') @@ -375,7 +375,7 @@ class Profile extends BaseCommand { log.info('profile', 'disabling tfa') - await npmProfile.set({ tfa: { password: password, mode: 'disable' } }, conf) + await set({ tfa: { password: password, mode: 'disable' } }, conf) if (this.npm.config.get('json')) { output.standard(JSON.stringify({ tfa: false }, null, 2)) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index f4072074898d3..3862f659bb508 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -7,7 +7,7 @@ const pacote = require('pacote') const npa = require('npm-package-arg') const npmFetch = require('npm-registry-fetch') const { redactLog: replaceInfo } = require('@npmcli/redact') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const { getContents, logTar } = require('../utils/tar.js') // for historical reasons, publishConfig in package.json can contain ANY config // keys that npm supports in .npmrc files and elsewhere. We *may* want to diff --git a/lib/commands/team.js b/lib/commands/team.js index 22af61863851a..c36b6ef023a26 100644 --- a/lib/commands/team.js +++ b/lib/commands/team.js @@ -1,7 +1,7 @@ const columns = require('cli-columns') const libteam = require('libnpmteam') const { output } = require('proc-log') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const BaseCommand = require('../base-cmd.js') class Team extends BaseCommand { diff --git a/lib/commands/token.js b/lib/commands/token.js index 24ca21a8e29ce..ae58891a566c2 100644 --- a/lib/commands/token.js +++ b/lib/commands/token.js @@ -1,6 +1,6 @@ const { log, output } = require('proc-log') -const profile = require('npm-profile') -const otplease = require('../utils/otplease.js') +const { listTokens, createToken, removeToken } = require('npm-profile') +const { otplease } = require('../utils/auth.js') const readUserInfo = require('../utils/read-user-info.js') const BaseCommand = require('../base-cmd.js') @@ -48,7 +48,7 @@ class Token extends BaseCommand { const json = this.npm.config.get('json') const parseable = this.npm.config.get('parseable') log.info('token', 'getting list') - const tokens = await profile.listTokens(this.npm.flatOptions) + const tokens = await listTokens(this.npm.flatOptions) if (json) { output.standard(JSON.stringify(tokens, null, 2)) return @@ -92,7 +92,7 @@ class Token extends BaseCommand { const toRemove = [] const opts = { ...this.npm.flatOptions } log.info('token', `removing ${toRemove.length} tokens`) - const tokens = await profile.listTokens(opts) + const tokens = await listTokens(opts) args.forEach(id => { const matches = tokens.filter(token => token.key.indexOf(id) === 0) if (matches.length === 1) { @@ -113,7 +113,7 @@ class Token extends BaseCommand { }) await Promise.all( toRemove.map(key => { - return otplease(this.npm, opts, c => profile.removeToken(key, c)) + return otplease(this.npm, opts, c => removeToken(key, c)) }) ) if (json) { @@ -137,7 +137,7 @@ class Token extends BaseCommand { const result = await otplease( this.npm, { ...this.npm.flatOptions }, - c => profile.createToken(password, readonly, validCIDR, c) + c => createToken(password, readonly, validCIDR, c) ) delete result.key delete result.updated diff --git a/lib/commands/unpublish.js b/lib/commands/unpublish.js index 47a5db8206244..4944888fe5aca 100644 --- a/lib/commands/unpublish.js +++ b/lib/commands/unpublish.js @@ -6,7 +6,7 @@ const { output, log } = require('proc-log') const pkgJson = require('@npmcli/package-json') const { flatten } = require('@npmcli/config/lib/definitions') const getIdentity = require('../utils/get-identity.js') -const otplease = require('../utils/otplease.js') +const { otplease } = require('../utils/auth.js') const BaseCommand = require('../base-cmd.js') const LAST_REMAINING_VERSION_ERROR = 'Refusing to delete the last version of the package. ' + diff --git a/lib/package-url-cmd.js b/lib/package-url-cmd.js index bcefd17af4492..c7ae32174fcb6 100644 --- a/lib/package-url-cmd.js +++ b/lib/package-url-cmd.js @@ -1,5 +1,5 @@ const pacote = require('pacote') -const openUrl = require('./utils/open-url.js') +const { openUrl } = require('./utils/open-url.js') const { log } = require('proc-log') const BaseCommand = require('./base-cmd.js') diff --git a/lib/utils/auth.js b/lib/utils/auth.js index 04ca455ceb526..deaff6bbebbaf 100644 --- a/lib/utils/auth.js +++ b/lib/utils/auth.js @@ -1,23 +1,43 @@ -const profile = require('npm-profile') +const { webAuthOpener, adduserWeb, loginWeb, loginCouch, adduserCouch } = require('npm-profile') const { log } = require('proc-log') -const openUrlPrompt = require('../utils/open-url-prompt.js') +const { createOpener } = require('../utils/open-url.js') const read = require('../utils/read-user-info.js') -const otplease = require('../utils/otplease.js') + +const otplease = async (npm, opts, fn) => { + try { + return await fn(opts) + } catch (err) { + if (!process.stdin.isTTY || !process.stdout.isTTY) { + throw err + } + + // web otp + if (err.code === 'EOTP' && err.body?.authUrl && err.body?.doneUrl) { + const otp = await webAuthOpener( + createOpener(npm, 'Authenticate your account at'), + err.body.authUrl, + err.body.doneUrl, + opts + ) + return await fn({ ...opts, otp }) + } + + // classic otp + if (err.code === 'EOTP' || (err.code === 'E401' && /one-time pass/.test(err.body))) { + const otp = await read.otp('This operation requires a one-time password.\nEnter OTP:') + return await fn({ ...opts, otp }) + } + + throw err + } +} const adduser = async (npm, { creds, ...opts }) => { const authType = npm.config.get('auth-type') let res if (authType === 'web') { try { - res = await profile.adduserWeb((url, emitter) => { - openUrlPrompt( - npm, - url, - 'Create your account at', - 'Press ENTER to open in the browser...', - emitter - ) - }, opts) + res = await adduserWeb(createOpener(npm, 'Create your account at'), opts) } catch (err) { if (err.code === 'ENYI') { log.verbose('web add user not supported, trying couch') @@ -35,9 +55,7 @@ const adduser = async (npm, { creds, ...opts }) => { // npm registry quirk: If you "add" an existing user with their current // password, it's effectively a login, and if that account has otp you'll // be prompted for it. - res = await otplease(npm, opts, (reqOpts) => - profile.adduserCouch(username, email, password, reqOpts) - ) + res = await otplease(npm, opts, (reqOpts) => adduserCouch(username, email, password, reqOpts)) } // We don't know the username if it was a web login, all we can reliably log is scope and registry @@ -56,15 +74,7 @@ const login = async (npm, { creds, ...opts }) => { let res if (authType === 'web') { try { - res = await profile.loginWeb((url, emitter) => { - openUrlPrompt( - npm, - url, - 'Login at', - 'Press ENTER to open in the browser...', - emitter - ) - }, opts) + res = await loginWeb(createOpener(npm, 'Login at'), opts) } catch (err) { if (err.code === 'ENYI') { log.verbose('web login not supported, trying couch') @@ -78,9 +88,7 @@ const login = async (npm, { creds, ...opts }) => { if (!res) { const username = await read.username('Username:', creds.username) const password = await read.password('Password:', creds.password) - res = await otplease(npm, opts, (reqOpts) => - profile.loginCouch(username, password, reqOpts) - ) + res = await otplease(npm, opts, (reqOpts) => loginCouch(username, password, reqOpts)) } // We don't know the username if it was a web login, all we can reliably log is scope and registry @@ -97,4 +105,5 @@ const login = async (npm, { creds, ...opts }) => { module.exports = { adduser, login, + otplease, } diff --git a/lib/utils/open-url-prompt.js b/lib/utils/open-url-prompt.js deleted file mode 100644 index 6f4d453a959d5..0000000000000 --- a/lib/utils/open-url-prompt.js +++ /dev/null @@ -1,66 +0,0 @@ -const readline = require('readline') -const { input, output } = require('proc-log') -const open = require('./open-url.js') - -function print (npm, title, url) { - const json = npm.config.get('json') - - const message = json ? JSON.stringify({ title, url }) : `${title}:\n${url}` - - output.standard(message) -} - -// Prompt to open URL in browser if possible -const promptOpen = async (npm, url, title, prompt, emitter) => { - const browser = npm.config.get('browser') - const isInteractive = process.stdin.isTTY === true && process.stdout.isTTY === true - - try { - if (!/^https?:$/.test(new URL(url).protocol)) { - throw new Error() - } - } catch (_) { - throw new Error('Invalid URL: ' + url) - } - - print(npm, title, url) - - if (browser === false || !isInteractive) { - return - } - - const rl = readline.createInterface({ - input: process.stdin, - output: process.stdout, - }) - - const tryOpen = await input.read(() => new Promise(resolve => { - rl.on('SIGINT', () => { - rl.close() - resolve('SIGINT') - }) - - rl.question(prompt, () => { - resolve(true) - }) - - if (emitter && emitter.addListener) { - emitter.addListener('abort', () => { - rl.close() - resolve(false) - }) - } - })) - - if (tryOpen === 'SIGINT') { - throw new Error('canceled') - } - - if (!tryOpen) { - return - } - - await open(npm, url, 'Browser unavailable. Please open the URL manually') -} - -module.exports = promptOpen diff --git a/lib/utils/open-url.js b/lib/utils/open-url.js index 46b7abc731fa1..7d8e1cb5a2d23 100644 --- a/lib/utils/open-url.js +++ b/lib/utils/open-url.js @@ -1,51 +1,95 @@ -const promiseSpawn = require('@npmcli/promise-spawn') -const { output } = require('proc-log') - +const { open } = require('@npmcli/promise-spawn') +const { output, input } = require('proc-log') const { URL } = require('url') +const readline = require('node:readline/promises') +const { once } = require('node:events') + +const assertValidUrl = (url) => { + try { + if (!/^https?:$/.test(new URL(url).protocol)) { + throw new Error() + } + } catch { + throw new Error('Invalid URL: ' + url) + } +} + +const outputMsg = (json, title, url) => { + const msg = json ? JSON.stringify({ title, url }) : `${title}:\n${url}` + output.standard(msg) +} // attempt to open URL in web-browser, print address otherwise: -const open = async (npm, url, errMsg, isFile) => { +const openUrl = async (npm, url, title, isFile) => { url = encodeURI(url) const browser = npm.config.get('browser') - - function printAlternateMsg () { - const json = npm.config.get('json') - const alternateMsg = json - ? JSON.stringify({ - title: errMsg, - url, - }, null, 2) - : `${errMsg}:\n ${url}\n` - - output.standard(alternateMsg) - } + const json = npm.config.get('json') if (browser === false) { - printAlternateMsg() + outputMsg(json, title, url) return } // We pass this in as true from the help command so we know we don't have to // check the protocol if (!isFile) { - try { - if (!/^https?:$/.test(new URL(url).protocol)) { - throw new Error() - } - } catch { - throw new Error('Invalid URL: ' + url) + assertValidUrl(url) + } + + try { + await input.start(() => open(url, { + command: browser === true ? null : browser, + })) + } catch (err) { + if (err.code !== 127) { + throw err } + outputMsg(json, title, url) } +} + +// Prompt to open URL in browser if possible +const openUrlPrompt = async (npm, url, title, prompt, { signal }) => { + const browser = npm.config.get('browser') + const json = npm.config.get('json') - const command = browser === true ? null : browser - await promiseSpawn.open(url, { command }) - .catch((err) => { - if (err.code !== 127) { - throw err - } + assertValidUrl(url) + outputMsg(json, title, url) + + if (browser === false || !process.stdin.isTTY || !process.stdout.isTTY) { + return + } - printAlternateMsg() - }) + const rl = readline.createInterface({ + input: process.stdin, + output: process.stdout, + }) + + try { + await input.read(() => Promise.race([ + rl.question(prompt, { signal }), + once(rl, 'error'), + once(rl, 'SIGINT').then(() => { + throw new Error('canceled') + }), + ])) + rl.close() + await openUrl(npm, url, 'Browser unavailable. Please open the URL manually') + } catch (err) { + rl.close() + if (err.name !== 'AbortError') { + throw err + } + } } -module.exports = open +// Rearrange arguments and return a function that takes the two arguments +// returned from the npm-profile methods that take an opener +const createOpener = (npm, title, prompt = 'Press ENTER to open in the browser...') => + (url, opts) => openUrlPrompt(npm, url, title, prompt, opts) + +module.exports = { + openUrl, + openUrlPrompt, + createOpener, +} diff --git a/lib/utils/otplease.js b/lib/utils/otplease.js deleted file mode 100644 index b8dd0b66ed766..0000000000000 --- a/lib/utils/otplease.js +++ /dev/null @@ -1,46 +0,0 @@ -async function otplease (npm, opts, fn) { - try { - return await fn(opts) - } catch (err) { - if (!process.stdin.isTTY || !process.stdout.isTTY) { - throw err - } - - if (isWebOTP(err)) { - const webAuth = require('./web-auth') - const openUrlPrompt = require('./open-url-prompt') - - const openerPromise = (url, emitter) => - openUrlPrompt( - npm, - url, - 'Authenticate your account at', - 'Press ENTER to open in the browser...', - emitter - ) - const otp = await webAuth(openerPromise, err.body.authUrl, err.body.doneUrl, opts) - return await fn({ ...opts, otp }) - } - - if (isClassicOTP(err)) { - const readUserInfo = require('./read-user-info.js') - const otp = await readUserInfo.otp('This operation requires a one-time password.\nEnter OTP:') - return await fn({ ...opts, otp }) - } - - throw err - } -} - -function isWebOTP (err) { - if (err.code === 'EOTP' && err.body) { - return err.body.authUrl && err.body.doneUrl - } - return false -} - -function isClassicOTP (err) { - return err.code === 'EOTP' || (err.code === 'E401' && /one-time pass/.test(err.body)) -} - -module.exports = otplease diff --git a/lib/utils/read-user-info.js b/lib/utils/read-user-info.js index 4e8def4bdf1de..a9a50f8263ff6 100644 --- a/lib/utils/read-user-info.js +++ b/lib/utils/read-user-info.js @@ -2,11 +2,6 @@ const { read: _read } = require('read') const userValidate = require('npm-user-validate') const { log, input } = require('proc-log') -exports.otp = readOTP -exports.password = readPassword -exports.username = readUsername -exports.email = readEmail - const otpPrompt = `This command requires a one-time password (OTP) from your authenticator app. Enter one below. You can also pass one on the command line by appending --otp=123456. For more information, see: @@ -63,3 +58,10 @@ function readEmail (msg = emailPrompt, email, isRetry) { return read({ prompt: msg, default: email || '' }) .then((username) => readEmail(msg, username, true)) } + +module.exports = { + otp: readOTP, + password: readPassword, + username: readUsername, + email: readEmail, +} diff --git a/lib/utils/web-auth.js b/lib/utils/web-auth.js deleted file mode 100644 index ce551687098fc..0000000000000 --- a/lib/utils/web-auth.js +++ /dev/null @@ -1,20 +0,0 @@ -const EventEmitter = require('events') -const { webAuthCheckLogin } = require('npm-profile') - -async function webAuth (opener, initialUrl, doneUrl, opts) { - const doneEmitter = new EventEmitter() - - const openPromise = opener(initialUrl, doneEmitter) - const webAuthCheckPromise = webAuthCheckLogin(doneUrl, { ...opts, cache: false }) - .then(authResult => { - // cancel open prompt if it's present - doneEmitter.emit('abort') - - return authResult.token - }) - - await openPromise - return await webAuthCheckPromise -} - -module.exports = webAuth diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index 558cb25e2674e..03787ea7cd4e6 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -231,7 +231,6 @@ class MockRegistry { this.nock = this.nock .post(this.fullPath('/-/v1/login'), body => { this.#tap.ok(body.create) // Sole difference from weblogin - this.#tap.ok(body.hostname) return true }) .reply(200, { doneUrl, loginUrl }) @@ -243,8 +242,7 @@ class MockRegistry { const doneUrl = new URL('/npm-cli-test/done', this.origin).href const loginUrl = new URL('/npm-cli-test/login', this.origin).href this.nock = this.nock - .post(this.fullPath('/-/v1/login'), body => { - this.#tap.ok(body.hostname) + .post(this.fullPath('/-/v1/login'), () => { return true }) .reply(200, { doneUrl, loginUrl }) diff --git a/tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs b/tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs deleted file mode 100644 index cf5feed44cc37..0000000000000 --- a/tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs +++ /dev/null @@ -1,22 +0,0 @@ -/* IMPORTANT - * This snapshot file is auto-generated, but designed for humans. - * It should be checked into source control and tracked carefully. - * Re-generate by setting TAP_SNAPSHOT=1 and running tests. - * Make sure to inspect the output below. Do not ignore changes! - */ -'use strict' -exports[`test/lib/utils/open-url-prompt.js TAP does not error when opener can not find command > Outputs extra Browser unavailable message and url 1`] = ` -npm home: -https://www.npmjs.com -Browser unavailable. Please open the URL manually: - https://www.npmjs.com -` - -exports[`test/lib/utils/open-url-prompt.js TAP opens a url > must match snapshot 1`] = ` -npm home: -https://www.npmjs.com -` - -exports[`test/lib/utils/open-url-prompt.js TAP prints json output > must match snapshot 1`] = ` -{"title":"npm home","url":"https://www.npmjs.com"} -` diff --git a/tap-snapshots/test/lib/utils/open-url.js.test.cjs b/tap-snapshots/test/lib/utils/open-url.js.test.cjs index f1560db686cde..92511b9284c7a 100644 --- a/tap-snapshots/test/lib/utils/open-url.js.test.cjs +++ b/tap-snapshots/test/lib/utils/open-url.js.test.cjs @@ -5,19 +5,33 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' -exports[`test/lib/utils/open-url.js TAP prints where to go when browser is disabled > printed expected message 1`] = ` +exports[`test/lib/utils/open-url.js TAP open url prints where to go when browser is disabled > printed expected message 1`] = ` npm home: - https://www.npmjs.com +https://www.npmjs.com ` -exports[`test/lib/utils/open-url.js TAP prints where to go when browser is disabled and json is enabled > printed expected message 1`] = ` -{ - "title": "npm home", - "url": "https://www.npmjs.com" -} +exports[`test/lib/utils/open-url.js TAP open url prints where to go when browser is disabled and json is enabled > printed expected message 1`] = ` +{"title":"npm home","url":"https://www.npmjs.com"} ` -exports[`test/lib/utils/open-url.js TAP prints where to go when given browser does not exist > printed expected message 1`] = ` +exports[`test/lib/utils/open-url.js TAP open url prints where to go when given browser does not exist > printed expected message 1`] = ` npm home: - https://www.npmjs.com +https://www.npmjs.com +` + +exports[`test/lib/utils/open-url.js TAP open url prompt does not error when opener can not find command > Outputs extra Browser unavailable message and url 1`] = ` +npm home: +https://www.npmjs.com + +Browser unavailable. Please open the URL manually: +https://www.npmjs.com +` + +exports[`test/lib/utils/open-url.js TAP open url prompt opens a url > must match snapshot 1`] = ` +npm home: +https://www.npmjs.com +` + +exports[`test/lib/utils/open-url.js TAP open url prompt prints json output > must match snapshot 1`] = ` +{"title":"npm home","url":"https://www.npmjs.com"} ` diff --git a/test/lib/commands/adduser.js b/test/lib/commands/adduser.js index 410e8c4987ca6..3063cdbeeab5e 100644 --- a/test/lib/commands/adduser.js +++ b/test/lib/commands/adduser.js @@ -9,9 +9,8 @@ const MockRegistry = require('@npmcli/mock-registry') const stream = require('stream') const mockAddUser = async (t, { stdin: stdinLines, registry: registryUrl, ...options } = {}) => { - let stdin if (stdinLines) { - stdin = new stream.PassThrough() + const stdin = new stream.PassThrough() for (const l of stdinLines) { stdin.write(l + '\n') } @@ -30,7 +29,6 @@ const mockAddUser = async (t, { stdin: stdinLines, registry: registryUrl, ...opt }) return { registry, - stdin, rc: () => ini.parse(fs.readFileSync(path.join(mock.home, '.npmrc'), 'utf8')), ...mock, } diff --git a/test/lib/commands/bugs.js b/test/lib/commands/bugs.js index e2ebfb5306574..c624ba3f2d91d 100644 --- a/test/lib/commands/bugs.js +++ b/test/lib/commands/bugs.js @@ -63,7 +63,7 @@ t.test('open bugs urls & emails', async t => { const { npm } = await loadMockNpm(t, { mocks: { pacote, - '{LIB}/utils/open-url.js': openUrl, + '{LIB}/utils/open-url.js': { openUrl }, }, }) diff --git a/test/lib/commands/docs.js b/test/lib/commands/docs.js index e11df6b07bc5e..2a7951edeaf76 100644 --- a/test/lib/commands/docs.js +++ b/test/lib/commands/docs.js @@ -84,7 +84,7 @@ const setup = async (t, { prefixDir = fixtures.pkg, config } = {}) => { const res = await mockNpm(t, { prefixDir, mocks: { - '{LIB}/utils/open-url.js': openUrl, + '{LIB}/utils/open-url.js': { openUrl }, }, config, }) diff --git a/test/lib/commands/repo.js b/test/lib/commands/repo.js index 114cdf919510a..f6e3ed753d039 100644 --- a/test/lib/commands/repo.js +++ b/test/lib/commands/repo.js @@ -187,9 +187,11 @@ const loadMockNpm = async (t, prefixDir, config = {}) => { const mock = await mockNpm(t, { command: 'repo', mocks: { - '{LIB}/utils/open-url.js': async (_, url) => { - opened[url] = opened[url] || 0 - opened[url]++ + '{LIB}/utils/open-url.js': { + openUrl: async (_, url) => { + opened[url] = opened[url] || 0 + opened[url]++ + }, }, }, config, diff --git a/test/lib/utils/otplease.js b/test/lib/utils/auth.js similarity index 87% rename from test/lib/utils/otplease.js rename to test/lib/utils/auth.js index 6dc3ee0f0b069..7d7f63d7ddfd5 100644 --- a/test/lib/utils/otplease.js +++ b/test/lib/utils/auth.js @@ -3,24 +3,21 @@ const setupMockNpm = require('../../fixtures/mock-npm') const tmock = require('../../fixtures/tmock') const setupOtplease = async (t, { otp = {}, ...rest }, fn) => { - const readUserInfo = { - otp: async () => '1234', - } - - const webAuth = async (opener) => { - opener() - return '1234' - } - - const otplease = tmock(t, '{LIB}/utils/otplease.js', { - '{LIB}/utils/read-user-info.js': readUserInfo, - '{LIB}/utils/open-url-prompt.js': () => {}, - '{LIB}/utils/web-auth': webAuth, + const { otplease } = tmock(t, '{LIB}/utils/auth.js', { + '{LIB}/utils/read-user-info.js': { + otp: async () => '1234', + }, + '{LIB}/utils/open-url.js': { + createOpener: () => () => {}, + }, + 'npm-profile': { + webAuthOpener: async (opener) => { + opener() + return '1234' + }, + }, }) - - const { npm } = await setupMockNpm(t, rest) - - return await otplease(npm, otp, fn) + return otplease(await setupMockNpm(t, rest).then(({ npm }) => npm), otp, fn) } t.test('returns function results on success', async (t) => { diff --git a/test/lib/utils/open-url-prompt.js b/test/lib/utils/open-url-prompt.js deleted file mode 100644 index 91058ec9e29a4..0000000000000 --- a/test/lib/utils/open-url-prompt.js +++ /dev/null @@ -1,156 +0,0 @@ -const t = require('tap') -const EventEmitter = require('events') -const tmock = require('../../fixtures/tmock') -const mockNpm = require('../../fixtures/mock-npm') - -const mockOpenUrlPrompt = async (t, { - questionShouldResolve = true, - openUrlPromptInterrupted = false, - openerResult = null, - isTTY = true, - emitter = null, - url: openUrl = 'https://www.npmjs.com', - ...config -}) => { - const mock = await mockNpm(t, { - globals: { - 'process.stdin.isTTY': isTTY, - 'process.stdout.isTTY': isTTY, - }, - config, - }) - - let openerUrl = null - let openerOpts = null - - const openUrlPrompt = tmock(t, '{LIB}/utils/open-url-prompt.js', { - '@npmcli/promise-spawn': { - open: async (url, options) => { - openerUrl = url - openerOpts = options - if (openerResult) { - throw openerResult - } - }, - }, - readline: { - createInterface: () => ({ - question: (_q, cb) => { - if (questionShouldResolve === true) { - cb() - } - }, - close: () => {}, - on: (_signal, cb) => { - if (openUrlPromptInterrupted && _signal === 'SIGINT') { - cb() - } - }, - }), - }, - }) - - let error - const args = [mock.npm, openUrl, 'npm home', 'prompt'] - if (emitter) { - mock.open = openUrlPrompt(...args, emitter) - } else { - await openUrlPrompt(...args).catch((er) => error = er) - } - - return { - ...mock, - openerUrl, - openerOpts, - OUTPUT: mock.joinedOutput(), - emitter, - error, - } -} - -t.test('does not open a url in non-interactive environments', async t => { - const { openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { isTTY: false }) - - t.equal(openerUrl, null, 'did not open') - t.same(openerOpts, null, 'did not open') -}) - -t.test('opens a url', async t => { - const { OUTPUT, openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { browser: true }) - - t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') - t.same(openerOpts, { command: null }, 'passed command as null (the default)') - t.matchSnapshot(OUTPUT) -}) - -t.test('opens a url with browser string', async t => { - const { openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { browser: 'firefox' }) - - t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') - // FIXME: browser string is parsed as a boolean in config layer - // this is a bug that should be fixed or the config should not allow it - t.same(openerOpts, { command: null }, 'passed command as null (the default)') -}) - -t.test('prints json output', async t => { - const { OUTPUT } = await mockOpenUrlPrompt(t, { json: true }) - - t.matchSnapshot(OUTPUT) -}) - -t.test('returns error for non-https url', async t => { - const { error, OUTPUT, openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { - url: 'ftp://www.npmjs.com', - }) - - t.match(error, /Invalid URL/, 'got the correct error') - t.equal(openerUrl, null, 'did not open') - t.same(openerOpts, null, 'did not open') - t.same(OUTPUT, '', 'printed no output') -}) - -t.test('does not open url if canceled', async t => { - const emitter = new EventEmitter() - const { openerUrl, openerOpts, open } = await mockOpenUrlPrompt(t, { - questionShouldResolve: false, - emitter, - }) - - emitter.emit('abort') - - await open - - t.equal(openerUrl, null, 'did not open') - t.same(openerOpts, null, 'did not open') -}) - -t.test('returns error when opener errors', async t => { - const { error, openerUrl } = await mockOpenUrlPrompt(t, { - openerResult: Object.assign(new Error('Opener failed'), { code: 1 }), - }) - - t.match(error, /Opener failed/, 'got the correct error') - t.equal(openerUrl, 'https://www.npmjs.com', 'did not open') -}) - -t.test('does not error when opener can not find command', async t => { - const { OUTPUT, error, openerUrl } = await mockOpenUrlPrompt(t, { - // openerResult: new Error('Opener failed'), - openerResult: Object.assign(new Error('Opener failed'), { code: 127 }), - }) - - t.notOk(error, 'Did not error') - t.equal(openerUrl, 'https://www.npmjs.com', 'did not open') - t.matchSnapshot(OUTPUT, 'Outputs extra Browser unavailable message and url') -}) - -t.test('throws "canceled" error on SIGINT', async t => { - const emitter = new EventEmitter() - const { open } = await mockOpenUrlPrompt(t, { - questionShouldResolve: false, - openUrlPromptInterrupted: true, - emitter, - }) - - await t.rejects(open, /canceled/, 'message is canceled') -}) diff --git a/test/lib/utils/open-url.js b/test/lib/utils/open-url.js index dab7b41b92f1f..452a09fac97e5 100644 --- a/test/lib/utils/open-url.js +++ b/test/lib/utils/open-url.js @@ -1,6 +1,7 @@ const t = require('tap') const tmock = require('../../fixtures/tmock') const mockNpm = require('../../fixtures/mock-npm') +const EventEmitter = require('events') const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => { let openerUrl = null @@ -16,7 +17,7 @@ const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => { const mock = await mockNpm(t, { config }) - const openUrl = tmock(t, '{LIB}/utils/open-url.js', { + const { openUrl } = tmock(t, '{LIB}/utils/open-url.js', { '@npmcli/promise-spawn': { open }, }) @@ -34,110 +35,275 @@ const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => { } } -t.test('opens a url', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home']) - t.equal(openerUrl(), 'https://www.npmjs.com', 'opened the given url') - t.same(openerOpts(), { command: null }, 'passed command as null (the default)') - t.same(joinedOutput(), '', 'printed no output') -}) +const mockOpenUrlPrompt = async (t, { + questionShouldResolve = true, + openUrlPromptInterrupted = false, + openerResult = null, + isTTY = true, + abort = false, + url: openUrl = 'https://www.npmjs.com', + ...config +}) => { + const mock = await mockNpm(t, { + globals: { + 'process.stdin.isTTY': isTTY, + 'process.stdout.isTTY': isTTY, + }, + config, + }) -t.test('returns error for non-https url', async t => { - const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) - await t.rejects( - openUrl('ftp://www.npmjs.com', 'npm home'), - /Invalid URL/, - 'got the correct error' - ) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.same(joinedOutput(), '', 'printed no output') -}) + let openerUrl = null + let openerOpts = null -t.test('returns error for file url', async t => { - const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) - await t.rejects( - openUrl('file:///usr/local/bin/ls', 'npm home'), - /Invalid URL/, - 'got the correct error' - ) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.same(joinedOutput(), '', 'printed no output') -}) + const { openUrlPrompt } = tmock(t, '{LIB}/utils/open-url.js', { + '@npmcli/promise-spawn': { + open: async (url, options) => { + openerUrl = url + openerOpts = options + if (openerResult) { + throw openerResult + } + }, + }, + 'node:readline/promises': { + createInterface: () => { + return Object.assign(new EventEmitter(), { + question: async (p, { signal } = {}) => { + if (questionShouldResolve !== true) { + await new Promise((res, rej) => { + if (signal) { + signal.addEventListener('abort', () => { + const err = new Error('abort') + err.name = 'AbortError' + rej(err) + }) + } + }) + } + }, + close: () => {}, + once: function (event, cb) { + if (openUrlPromptInterrupted && event === 'SIGINT') { + cb() + } + }, + }) + }, + }, + }) -t.test('file url allowed if explicitly asked for', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['file:///man/page/npm-install', 'npm home', true]) - t.equal(openerUrl(), 'file:///man/page/npm-install', 'opened the given url') - t.same(openerOpts(), { command: null }, 'passed command as null (the default)') - t.same(joinedOutput(), '', 'printed no output') -}) + let error + const abortController = new AbortController() + const args = [mock.npm, openUrl, 'npm home', 'prompt', { signal: abortController.signal }] + if (abort) { + mock.open = openUrlPrompt(...args) + } else { + await openUrlPrompt(...args).catch((er) => error = er) + } -t.test('returns error for non-parseable url', async t => { - const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) - await t.rejects( - openUrl('git+ssh://user@host:repo.git', 'npm home'), - /Invalid URL/, - 'got the correct error' - ) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.same(joinedOutput(), '', 'printed no output') -}) + return { + ...mock, + openerUrl, + openerOpts, + OUTPUT: mock.joinedOutput(), + error, + abortController, + } +} -t.test('encodes non-URL-safe characters in url provided', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com/|cat', 'npm home']) - t.equal(openerUrl(), 'https://www.npmjs.com/%7Ccat', 'opened the encoded url') - t.same(openerOpts(), { command: null }, 'passed command as null (the default)') - t.same(joinedOutput(), '', 'printed no output') -}) +t.test('open url prompt', async t => { + t.test('does not open a url in non-interactive environments', async t => { + const { openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { isTTY: false }) -t.test('opens a url with the given browser', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home'], { browser: 'chrome' }) - t.equal(openerUrl(), 'https://www.npmjs.com', 'opened the given url') - // FIXME: browser string is parsed as a boolean in config layer - // this is a bug that should be fixed or the config should not allow it - t.same(openerOpts(), { command: null }, 'passed the given browser as command') - t.same(joinedOutput(), '', 'printed no output') -}) + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + }) -t.test('prints where to go when browser is disabled', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home'], { browser: false }) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.matchSnapshot(joinedOutput(), 'printed expected message') -}) + t.test('opens a url', async t => { + const { OUTPUT, openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { browser: true }) -t.test('prints where to go when browser is disabled and json is enabled', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home'], { browser: false, json: true }) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.matchSnapshot(joinedOutput(), 'printed expected message') -}) + t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') + t.same(openerOpts, { command: null }, 'passed command as null (the default)') + t.matchSnapshot(OUTPUT) + }) -t.test('prints where to go when given browser does not exist', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home'], - { - openerResult: Object.assign(new Error('failed'), { code: 127 }), - } - ) + t.test('opens a url with browser string', async t => { + const { openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { browser: 'firefox' }) + + t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') + // FIXME: browser string is parsed as a boolean in config layer + // this is a bug that should be fixed or the config should not allow it + t.same(openerOpts, { command: null }, 'passed command as null (the default)') + }) + + t.test('prints json output', async t => { + const { OUTPUT } = await mockOpenUrlPrompt(t, { json: true }) + + t.matchSnapshot(OUTPUT) + }) + + t.test('returns error for non-https url', async t => { + const { error, OUTPUT, openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { + url: 'ftp://www.npmjs.com', + }) + + t.match(error, /Invalid URL/, 'got the correct error') + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + t.same(OUTPUT, '', 'printed no output') + }) + + t.test('does not open url if canceled', async t => { + const { openerUrl, openerOpts, open, abortController } = await mockOpenUrlPrompt(t, { + questionShouldResolve: false, + abort: true, + }) + + abortController.abort() + + await open - t.equal(openerUrl(), 'https://www.npmjs.com', 'tried to open the correct url') - t.same(openerOpts(), { command: null }, 'tried to use the correct browser') - t.matchSnapshot(joinedOutput(), 'printed expected message') + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + }) + + t.test('returns error when opener errors', async t => { + const { error, openerUrl } = await mockOpenUrlPrompt(t, { + openerResult: Object.assign(new Error('Opener failed'), { code: 1 }), + }) + + t.match(error, /Opener failed/, 'got the correct error') + t.equal(openerUrl, 'https://www.npmjs.com', 'did not open') + }) + + t.test('does not error when opener can not find command', async t => { + const { OUTPUT, error, openerUrl } = await mockOpenUrlPrompt(t, { + // openerResult: new Error('Opener failed'), + openerResult: Object.assign(new Error('Opener failed'), { code: 127 }), + }) + + t.notOk(error, 'Did not error') + t.equal(openerUrl, 'https://www.npmjs.com', 'did not open') + t.matchSnapshot(OUTPUT, 'Outputs extra Browser unavailable message and url') + }) + + t.test('throws "canceled" error on SIGINT', async t => { + const { open } = await mockOpenUrlPrompt(t, { + questionShouldResolve: false, + openUrlPromptInterrupted: true, + abort: true, + }) + + await t.rejects(open, /canceled/, 'message is canceled') + }) }) -t.test('handles unknown opener error', async t => { - const { openUrl } = await mockOpenUrl(t, null, { - browser: 'firefox', - openerResult: Object.assign(new Error('failed'), { code: 'ENOBRIAN' }), +t.test('open url', async t => { + t.test('opens a url', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home']) + t.equal(openerUrl(), 'https://www.npmjs.com', 'opened the given url') + t.same(openerOpts(), { command: null }, 'passed command as null (the default)') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('returns error for non-https url', async t => { + const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) + await t.rejects( + openUrl('ftp://www.npmjs.com', 'npm home'), + /Invalid URL/, + 'got the correct error' + ) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('returns error for file url', async t => { + const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) + await t.rejects( + openUrl('file:///usr/local/bin/ls', 'npm home'), + /Invalid URL/, + 'got the correct error' + ) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('file url allowed if explicitly asked for', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['file:///man/page/npm-install', 'npm home', true]) + t.equal(openerUrl(), 'file:///man/page/npm-install', 'opened the given url') + t.same(openerOpts(), { command: null }, 'passed command as null (the default)') + t.same(joinedOutput(), '', 'printed no output') }) - await t.rejects(openUrl('https://www.npmjs.com', 'npm home'), 'failed', 'got the correct error') + t.test('returns error for non-parseable url', async t => { + const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) + await t.rejects( + openUrl('git+ssh://user@host:repo.git', 'npm home'), + /Invalid URL/, + 'got the correct error' + ) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('encodes non-URL-safe characters in url provided', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com/|cat', 'npm home']) + t.equal(openerUrl(), 'https://www.npmjs.com/%7Ccat', 'opened the encoded url') + t.same(openerOpts(), { command: null }, 'passed command as null (the default)') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('opens a url with the given browser', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home'], { browser: 'chrome' }) + t.equal(openerUrl(), 'https://www.npmjs.com', 'opened the given url') + // FIXME: browser string is parsed as a boolean in config layer + // this is a bug that should be fixed or the config should not allow it + t.same(openerOpts(), { command: null }, 'passed the given browser as command') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('prints where to go when browser is disabled', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home'], { browser: false }) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.matchSnapshot(joinedOutput(), 'printed expected message') + }) + + t.test('prints where to go when browser is disabled and json is enabled', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home'], { browser: false, json: true }) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.matchSnapshot(joinedOutput(), 'printed expected message') + }) + + t.test('prints where to go when given browser does not exist', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home'], + { + openerResult: Object.assign(new Error('failed'), { code: 127 }), + } + ) + + t.equal(openerUrl(), 'https://www.npmjs.com', 'tried to open the correct url') + t.same(openerOpts(), { command: null }, 'tried to use the correct browser') + t.matchSnapshot(joinedOutput(), 'printed expected message') + }) + + t.test('handles unknown opener error', async t => { + const { openUrl } = await mockOpenUrl(t, null, { + browser: 'firefox', + openerResult: Object.assign(new Error('failed'), { code: 'ENOBRIAN' }), + }) + + await t.rejects(openUrl('https://www.npmjs.com', 'npm home'), 'failed', 'got the correct error') + }) }) diff --git a/test/lib/utils/web-auth.js b/test/lib/utils/web-auth.js deleted file mode 100644 index ec8c1d17e9fa1..0000000000000 --- a/test/lib/utils/web-auth.js +++ /dev/null @@ -1,33 +0,0 @@ -const t = require('tap') -const tmock = require('../../fixtures/tmock') - -const webAuthCheckLogin = async () => { - return { token: 'otp-token' } -} - -const webauth = tmock(t, '{LIB}/utils/web-auth.js', { - 'npm-profile': { webAuthCheckLogin }, -}) - -const initialUrl = 'https://example.com/auth' -const doneUrl = 'https://example.com/done' -const opts = {} - -t.test('returns token on success', async (t) => { - const opener = async () => {} - const result = await webauth(opener, initialUrl, doneUrl, opts) - t.equal(result, 'otp-token') -}) - -t.test('closes opener when auth check finishes', async (t) => { - const opener = (_url, emitter) => { - return new Promise((resolve) => { - // the only way to finish this promise is to emit abort on the emitter - emitter.addListener('abort', () => { - resolve() - }) - }) - } - const result = await webauth(opener, initialUrl, doneUrl, opts) - t.equal(result, 'otp-token') -})