feat: Support pure web authentication for commands

jumoel · ljharb · hfaulds · fritzy · commit c8bdb4a2517c · 2022-07-20T13:26:26.000-07:00
* feat: Add support for web auth, utilizing code from npm-profile

Co-authored-by: Jordan Harband &lt;ljharb@gmail.com&gt;
Co-authored-by: Hayden Faulds &lt;fauldsh@gmail.com&gt;
Co-authored-by: Sandeep Meduru &lt;sandeepmeduru@github.com&gt;
diff --git a/lib/auth/sso.js b/lib/auth/sso.js
@@ -52,7 +52,7 @@ const login = async (npm, { creds, registry, scope }) => {
     authType: ssoType,
   }
 
-  const { token, sso } = await otplease(opts,
+  const { token, sso } = await otplease(npm, opts,
     opts => profile.loginCouch(auth.username, auth.password, opts)
   )
 
diff --git a/lib/commands/access.js b/lib/commands/access.js
@@ -180,7 +180,7 @@ class Access extends BaseCommand {
 
   modifyPackage (pkg, opts, fn, requireScope = true) {
     return this.getPackage(pkg, requireScope)
-      .then(pkgName => otplease(opts, opts => fn(pkgName, opts)))
+      .then(pkgName => otplease(this.npm, opts, opts => fn(pkgName, opts)))
   }
 
   async getPackage (name, requireScope) {
diff --git a/lib/commands/deprecate.js b/lib/commands/deprecate.js
@@ -60,7 +60,7 @@ class Deprecate extends BaseCommand {
         packument.versions[v].deprecated = msg
       })
 
-    return otplease(this.npm.flatOptions, opts => fetch(uri, {
+    return otplease(this.npm, this.npm.flatOptions, opts => fetch(uri, {
       ...opts,
       spec: p,
       method: 'PUT',
diff --git a/lib/commands/dist-tag.js b/lib/commands/dist-tag.js
@@ -116,7 +116,7 @@ class DistTag extends BaseCommand {
       },
       spec,
     }
-    await otplease(reqOpts, reqOpts => regFetch(url, reqOpts))
+    await otplease(this.npm, reqOpts, reqOpts => regFetch(url, reqOpts))
     this.npm.output(`+${t}: ${spec.name}@${version}`)
   }
 
@@ -142,7 +142,7 @@ class DistTag extends BaseCommand {
       method: 'DELETE',
       spec,
     }
-    await otplease(reqOpts, reqOpts => regFetch(url, reqOpts))
+    await otplease(this.npm, reqOpts, reqOpts => regFetch(url, reqOpts))
     this.npm.output(`-${tag}: ${spec.name}@${version}`)
   }
 
diff --git a/lib/commands/hook.js b/lib/commands/hook.js
@@ -22,7 +22,7 @@ class Hook extends BaseCommand {
   static ignoreImplicitWorkspace = true
 
   async exec (args) {
-    return otplease({
+    return otplease(this.npm, {
       ...this.npm.flatOptions,
     }, (opts) => {
       switch (args[0]) {
diff --git a/lib/commands/org.js b/lib/commands/org.js
@@ -33,7 +33,7 @@ class Org extends BaseCommand {
   }
 
   async exec ([cmd, orgname, username, role], cb) {
-    return otplease({
+    return otplease(this.npm, {
       ...this.npm.flatOptions,
     }, opts => {
       switch (cmd) {
diff --git a/lib/commands/owner.js b/lib/commands/owner.js
@@ -202,7 +202,7 @@ class Owner extends BaseCommand {
 
     const dataPath = `/${spec.escapedName}/-rev/${encodeURIComponent(data._rev)}`
     try {
-      const res = await otplease(this.npm.flatOptions, opts => {
+      const res = await otplease(this.npm, this.npm.flatOptions, opts => {
         return npmFetch.json(dataPath, {
           ...opts,
           method: 'PUT',
diff --git a/lib/commands/profile.js b/lib/commands/profile.js
@@ -221,7 +221,7 @@ class Profile extends BaseCommand {
 
     newUser[prop] = value
 
-    const result = await otplease(conf, conf => npmProfile.set(newUser, conf))
+    const result = await otplease(this.npm, conf, conf => npmProfile.set(newUser, conf))
 
     if (this.npm.config.get('json')) {
       this.npm.output(JSON.stringify({ [prop]: result[prop] }, null, 2))
diff --git a/lib/commands/publish.js b/lib/commands/publish.js
@@ -62,6 +62,7 @@ class Publish extends BaseCommand {
     }
 
     const opts = { ...this.npm.flatOptions, progress: false }
+    log.disableProgress()
 
     // you can publish name@version, ./foo.tgz, etc.
     // even though the default is the 'file:.' cwd.
@@ -116,7 +117,7 @@ class Publish extends BaseCommand {
     log.notice('', `Publishing to ${outputRegistry}${dryRun ? ' (dry-run)' : ''}`)
 
     if (!dryRun) {
-      await otplease(opts, opts => libpub(manifest, tarballData, opts))
+      await otplease(this.npm, opts, opts => libpub(manifest, tarballData, opts))
     }
 
     if (spec.type === 'directory' && !ignoreScripts) {
diff --git a/lib/commands/team.js b/lib/commands/team.js
@@ -44,7 +44,7 @@ class Team extends BaseCommand {
     // XXX: "description" option to libnpmteam is used as a description of the
     // team, but in npm's options, this is a boolean meaning "show the
     // description in npm search output".  Hence its being set to null here.
-    await otplease({ ...this.npm.flatOptions }, opts => {
+    await otplease(this.npm, { ...this.npm.flatOptions }, opts => {
       entity = entity.replace(/^@/, '')
       switch (cmd) {
         case 'create': return this.create(entity, opts)
diff --git a/lib/commands/token.js b/lib/commands/token.js
@@ -121,7 +121,7 @@ class Token extends BaseCommand {
     })
     await Promise.all(
       toRemove.map(key => {
-        return otplease(conf, conf => {
+        return otplease(this.npm, conf, conf => {
           return profile.removeToken(key, conf)
         })
       })
@@ -146,7 +146,7 @@ class Token extends BaseCommand {
         const validCIDR = this.validateCIDRList(cidr)
         log.info('token', 'creating')
         return pulseTillDone.withPromise(
-          otplease(conf, conf => {
+          otplease(this.npm, conf, conf => {
             return profile.createToken(password, readonly, validCIDR, conf)
           })
         )
diff --git a/lib/commands/unpublish.js b/lib/commands/unpublish.js
@@ -130,7 +130,7 @@ class Unpublish extends BaseCommand {
     }
 
     if (!dryRun) {
-      await otplease(opts, opts => libunpub(spec, opts))
+      await otplease(this.npm, opts, opts => libunpub(spec, opts))
     }
     if (!silent) {
       this.npm.output(`- ${pkgName}${pkgVersion}`)
diff --git a/lib/utils/otplease.js b/lib/utils/otplease.js
@@ -1,17 +1,46 @@
-async function otplease (opts, fn) {
+async function otplease (npm, opts, fn) {
   try {
     return await fn(opts)
   } catch (err) {
-    const readUserInfo = require('./read-user-info.js')
-    if (err.code !== 'EOTP' && (err.code !== 'E401' || !/one-time pass/.test(err.body))) {
+    if (!process.stdin.isTTY || !process.stdout.isTTY) {
       throw err
-    } else if (!process.stdin.isTTY || !process.stdout.isTTY) {
-      throw err
-    } else {
+    }
+
+    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 false
   }
+  return err.body.authUrl && err.body.doneUrl
+}
+
+function isClassicOTP (err) {
+  return err.code === 'EOTP' || (err.code === 'E401' && /one-time pass/.test(err.body))
 }
 
 module.exports = otplease
diff --git a/lib/utils/web-auth.js b/lib/utils/web-auth.js
@@ -0,0 +1,20 @@
+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/test/lib/utils/otplease.js b/test/lib/utils/otplease.js
@@ -1,17 +1,25 @@
 const t = require('tap')
+
+const { fake: mockNpm } = require('../../fixtures/mock-npm')
 const mockGlobals = require('../../fixtures/mock-globals')
 
 const readUserInfo = {
   otp: async () => '1234',
 }
+const webAuth = async (opener) => {
+  opener()
+  return '1234'
+}
 
 const otplease = t.mock('../../../lib/utils/otplease.js', {
   '../../../lib/utils/read-user-info.js': readUserInfo,
+  '../../../lib/utils/open-url-prompt.js': () => {},
+  '../../../lib/utils/web-auth': webAuth,
 })
 
 t.test('returns function results on success', async (t) => {
   const fn = () => 'test string'
-  const result = await otplease({}, fn)
+  const result = await otplease(null, {}, fn)
   t.equal('test string', result)
 })
 
@@ -26,7 +34,7 @@ t.test('returns function results on otp success', async (t) => {
     }
     throw Object.assign(new Error('nope'), { code: 'EOTP' })
   }
-  const result = await otplease({}, fn)
+  const result = await otplease(null, {}, fn)
   t.equal('success', result)
 })
 
@@ -51,7 +59,31 @@ t.test('prompts for otp for EOTP', async (t) => {
     t.end()
   }
 
-  await otplease({ some: 'prop' }, fn)
+  await otplease(null, { some: 'prop' }, fn)
+})
+
+t.test('returns function results on webauth success', async (t) => {
+  mockGlobals(t, {
+    'process.stdin': { isTTY: true },
+    'process.stdout': { isTTY: true },
+  })
+
+  const npm = mockNpm({ config: { browser: 'firefox' } })
+  const fn = ({ otp }) => {
+    if (otp) {
+      return 'success'
+    }
+    throw Object.assign(new Error('nope'), {
+      code: 'EOTP',
+      body: {
+        authUrl: 'https://www.example.com/auth',
+        doneUrl: 'https://www.example.com/done',
+      },
+    })
+  }
+
+  const result = await otplease(npm, {}, fn)
+  t.equal('success', result)
 })
 
 t.test('prompts for otp for 401', async (t) => {
@@ -78,7 +110,7 @@ t.test('prompts for otp for 401', async (t) => {
     t.end()
   }
 
-  await otplease({ some: 'prop' }, fn)
+  await otplease(null, { some: 'prop' }, fn)
 })
 
 t.test('does not prompt for non-otp errors', async (t) => {
@@ -95,7 +127,11 @@ t.test('does not prompt for non-otp errors', async (t) => {
     throw new Error('nope')
   }
 
-  t.rejects(otplease({ some: 'prop' }, fn), { message: 'nope' }, 'rejects with the original error')
+  t.rejects(
+    otplease(null, { some: 'prop' }, fn),
+    { message: 'nope' },
+    'rejects with the original error'
+  )
 })
 
 t.test('does not prompt if stdin or stdout is not a tty', async (t) => {
@@ -112,5 +148,9 @@ t.test('does not prompt if stdin or stdout is not a tty', async (t) => {
     throw Object.assign(new Error('nope'), { code: 'EOTP' })
   }
 
-  t.rejects(otplease({ some: 'prop' }, fn), { message: 'nope' }, 'rejects with the original error')
+  t.rejects(
+    otplease(null, { some: 'prop' }, fn),
+    { message: 'nope' },
+    'rejects with the original error'
+  )
 })
diff --git a/test/lib/utils/web-auth.js b/test/lib/utils/web-auth.js
@@ -0,0 +1,32 @@
+const t = require('tap')
+
+const webAuthCheckLogin = async () => {
+  return { token: 'otp-token' }
+}
+
+const webauth = t.mock('../../../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, reject) => {
+      // the only way to finish this promise is to emit aboter on the emitter
+      emitter.addListener('abort', () => {
+        resolve()
+      })
+    })
+  }
+  const result = await webauth(opener, initialUrl, doneUrl, opts)
+  t.equal(result, 'otp-token')
+})

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ const login = async (npm, { creds, registry, scope }) => {`
`52`	`52`	`authType: ssoType,`
`53`	`53`	`}`
`54`	`54`
`55`		`- const { token, sso } = await otplease(opts,`
	`55`	`+ const { token, sso } = await otplease(npm, opts,`
`56`	`56`	`opts => profile.loginCouch(auth.username, auth.password, opts)`
`57`	`57`	`)`
`58`	`58`
Original file line number	Diff line number	Diff line change
`@@ -180,7 +180,7 @@ class Access extends BaseCommand {`
`180`	`180`
`181`	`181`	`modifyPackage (pkg, opts, fn, requireScope = true) {`
`182`	`182`	`return this.getPackage(pkg, requireScope)`
`183`		`- .then(pkgName => otplease(opts, opts => fn(pkgName, opts)))`
	`183`	`+ .then(pkgName => otplease(this.npm, opts, opts => fn(pkgName, opts)))`
`184`	`184`	`}`
`185`	`185`
`186`	`186`	`async getPackage (name, requireScope) {`
Original file line number	Diff line number	Diff line change
`@@ -116,7 +116,7 @@ class DistTag extends BaseCommand {`
`116`	`116`	`},`
`117`	`117`	`spec,`
`118`	`118`	`}`
`119`		`- await otplease(reqOpts, reqOpts => regFetch(url, reqOpts))`
	`119`	`+ await otplease(this.npm, reqOpts, reqOpts => regFetch(url, reqOpts))`
`120`	`120`	this.npm.output(`+${t}: ${spec.name}@${version}`)
`121`	`121`	`}`
`122`	`122`
`@@ -142,7 +142,7 @@ class DistTag extends BaseCommand {`
`142`	`142`	`method: 'DELETE',`
`143`	`143`	`spec,`
`144`	`144`	`}`
`145`		`- await otplease(reqOpts, reqOpts => regFetch(url, reqOpts))`
	`145`	`+ await otplease(this.npm, reqOpts, reqOpts => regFetch(url, reqOpts))`
`146`	`146`	this.npm.output(`-${tag}: ${spec.name}@${version}`)
`147`	`147`	`}`
`148`	`148`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ class Org extends BaseCommand {`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`async exec ([cmd, orgname, username, role], cb) {`
`36`		`- return otplease({`
	`36`	`+ return otplease(this.npm, {`
`37`	`37`	`...this.npm.flatOptions,`
`38`	`38`	`}, opts => {`
`39`	`39`	`switch (cmd) {`
Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ class Publish extends BaseCommand {`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`const opts = { ...this.npm.flatOptions, progress: false }`
	`65`	`+ log.disableProgress()`
`65`	`66`
`66`	`67`	`// you can publish name@version, ./foo.tgz, etc.`
`67`	`68`	`// even though the default is the 'file:.' cwd.`
`@@ -116,7 +117,7 @@ class Publish extends BaseCommand {`
`116`	`117`	log.notice('', `Publishing to ${outputRegistry}${dryRun ? ' (dry-run)' : ''}`)
`117`	`118`
`118`	`119`	`if (!dryRun) {`
`119`		`- await otplease(opts, opts => libpub(manifest, tarballData, opts))`
	`120`	`+ await otplease(this.npm, opts, opts => libpub(manifest, tarballData, opts))`
`120`	`121`	`}`
`121`	`122`
`122`	`123`	`if (spec.type === 'directory' && !ignoreScripts) {`