feat: prompt before opening web-login URL when performing login/adduser (#4960)

Prompt before opening web-login URL when performing login/adduser

Author: jumoel

lib/auth/legacy.js

@@ -1,6 +1,6 @@
 const profile = require('npm-profile')
 const log = require('../utils/log-shim')
-const openUrl = require('../utils/open-url.js')
+const openUrlPrompt = require('../utils/open-url-prompt.js')
 const read = require('../utils/read-user-info.js')
 
 const loginPrompter = async (creds) => {
@@ -47,7 +47,15 @@ const login = async (npm, opts) => {
     return newUser
   }
 
-  const openerPromise = (url) => openUrl(npm, url, 'to complete your login please visit')
+  const openerPromise = (url, emitter) =>
+    openUrlPrompt(
+      npm,
+      url,
+      'Authenticate your account at',
+      'Press ENTER to open in the browser...',
+      emitter
+    )
+
   try {
     res = await profile.login(openerPromise, loginPrompter, opts)
   } catch (err) {

lib/utils/open-url-prompt.js

@@ -0,0 +1,69 @@
+const readline = require('readline')
+const opener = require('opener')
+
+function print (npm, title, url) {
+  const json = npm.config.get('json')
+
+  const message = json ? JSON.stringify({ title, url }) : `${title}:\n${url}`
+
+  npm.output(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 new Promise(resolve => {
+    rl.question(prompt, () => {
+      resolve(true)
+    })
+
+    if (emitter && emitter.addListener) {
+      emitter.addListener('abort', () => {
+        rl.close()
+
+        // clear the prompt line
+        npm.output('')
+
+        resolve(false)
+      })
+    }
+  })
+
+  if (!tryOpen) {
+    return
+  }
+
+  const command = browser === true ? null : browser
+  await new Promise((resolve, reject) => {
+    opener(url, { command }, err => {
+      if (err) {
+        return reject(err)
+      }
+
+      return resolve()
+    })
+  })
+}
+
+module.exports = promptOpen

tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs

@@ -0,0 +1,25 @@
+/* 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 opens a url > must match snapshot 1`] = `
+Array [
+  Array [
+    String(
+      npm home:
+      https://www.npmjs.com
+    ),
+  ],
+]
+`
+
+exports[`test/lib/utils/open-url-prompt.js TAP prints json output > must match snapshot 1`] = `
+Array [
+  Array [
+    "{\\"title\\":\\"npm home\\",\\"url\\":\\"https://www.npmjs.com\\"}",
+  ],
+]
+`

test/lib/auth/legacy.js

@@ -12,7 +12,7 @@ const legacy = t.mock('../../../lib/auth/legacy.js', {
     },
   },
   'npm-profile': profile,
-  '../../../lib/utils/open-url.js': (npm, url, msg) => {
+  '../../../lib/utils/open-url-prompt.js': (_npm, url) => {
     if (!url) {
       throw Object.assign(new Error('failed open url'), { code: 'ERROR' })
     }

test/lib/utils/open-url-prompt.js

@@ -0,0 +1,150 @@
+const t = require('tap')
+const mockGlobals = require('../../fixtures/mock-globals.js')
+const EventEmitter = require('events')
+
+const OUTPUT = []
+const output = (...args) => OUTPUT.push(args)
+const npm = {
+  _config: {
+    json: false,
+    browser: true,
+  },
+  config: {
+    get: k => npm._config[k],
+    set: (k, v) => {
+      npm._config[k] = v
+    },
+  },
+  output,
+}
+
+let openerUrl = null
+let openerOpts = null
+let openerResult = null
+const opener = (url, opts, cb) => {
+  openerUrl = url
+  openerOpts = opts
+  return cb(openerResult)
+}
+
+let questionShouldResolve = true
+const readline = {
+  createInterface: () => ({
+    question: (_q, cb) => {
+      if (questionShouldResolve === true) {
+        cb()
+      }
+    },
+    close: () => {},
+  }),
+}
+
+const openUrlPrompt = t.mock('../../../lib/utils/open-url-prompt.js', {
+  opener,
+  readline,
+})
+
+mockGlobals(t, {
+  'process.stdin.isTTY': true,
+  'process.stdout.isTTY': true,
+})
+
+t.test('does not open a url in non-interactive environments', async t => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+  })
+
+  mockGlobals(t, {
+    'process.stdin.isTTY': false,
+    'process.stdout.isTTY': false,
+  })
+
+  await openUrlPrompt(npm, 'https://www.npmjs.com', 'npm home', 'prompt')
+  t.equal(openerUrl, null, 'did not open')
+  t.same(openerOpts, null, 'did not open')
+})
+
+t.test('opens a url', async t => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+    npm._config.browser = true
+  })
+
+  npm._config.browser = 'browser'
+  await openUrlPrompt(npm, 'https://www.npmjs.com', 'npm home', 'prompt')
+  t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url')
+  t.same(openerOpts, { command: 'browser' }, 'passed command as null (the default)')
+  t.matchSnapshot(OUTPUT)
+})
+
+t.test('prints json output', async t => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+    npm._config.json = false
+  })
+
+  npm._config.json = true
+  await openUrlPrompt(npm, 'https://www.npmjs.com', 'npm home', 'prompt')
+  t.matchSnapshot(OUTPUT)
+})
+
+t.test('returns error for non-https url', async t => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+  })
+  await t.rejects(
+    openUrlPrompt(npm, 'ftp://www.npmjs.com', 'npm home', 'prompt'),
+    /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 => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+    questionShouldResolve = true
+  })
+
+  questionShouldResolve = false
+  const emitter = new EventEmitter()
+
+  const open = openUrlPrompt(npm, 'https://www.npmjs.com', 'npm home', 'prompt', 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 => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    openerResult = null
+    OUTPUT.length = 0
+  })
+
+  openerResult = new Error('Opener failed')
+
+  await t.rejects(
+    openUrlPrompt(npm, 'https://www.npmjs.com', 'npm home', 'prompt'),
+    /Opener failed/,
+    'got the correct error'
+  )
+  t.equal(openerUrl, 'https://www.npmjs.com', 'did not open')
+})