chore: refactor adduser

- Refactored lib/auth modules
  - Removed some of the extra wrappers from cb to promises
  - Replaced impl to favor usage of async/await
- Added test/lib/auth unit tests

PR-URL: #1664
Credit: @ruyadorno
Close: #1664
Reviewed-by: @isaacs

Author: ruyadorno

lib/adduser.js

@@ -1,6 +1,15 @@
+'use strict'
+
 const log = require('npmlog')
 const npm = require('./npm.js')
+const output = require('./utils/output.js')
 const usageUtil = require('./utils/usage.js')
+const authTypes = {
+  legacy: require('./auth/legacy.js'),
+  oauth: require('./auth/oauth.js'),
+  saml: require('./auth/saml.js'),
+  sso: require('./auth/sso.js')
+}
 
 const usage = usageUtil(
   'adduser',
@@ -11,8 +20,7 @@ const completion = require('./utils/completion/none.js')
 
 const cmd = (args, cb) => adduser(args).then(() => cb()).catch(cb)
 
-const getRegistry = opts => {
-  const { scope, registry } = opts
+const getRegistry = ({ scope, registry }) => {
   if (scope) {
     const scopedRegistry = npm.config.get(`${scope}:registry`)
     const cliRegistry = npm.config.get('registry', 'cli')
@@ -24,37 +32,51 @@ const getRegistry = opts => {
 }
 
 const getAuthType = ({ authType }) => {
-  try {
-    return require('./auth/' + authType)
-  } catch (e) {
+  const type = authTypes[authType]
+
+  if (!type) {
     throw new Error('no such auth module')
   }
+
+  return type
 }
 
-const adduser = async args => {
-  const registry = getRegistry(npm.flatOptions)
+const saveConfig = () => new Promise((resolve, reject) => {
+  npm.config.save('user', er => er ? reject(er) : resolve())
+})
+
+const updateConfig = async ({ newCreds, registry, scope }) => {
+  npm.config.del('_token', 'user') // prevent legacy pollution
+
+  if (scope) {
+    npm.config.set(scope + ':registry', registry, 'user')
+  }
+
+  npm.config.setCredentialsByURI(registry, newCreds)
+  await saveConfig()
+}
+
+const adduser = async (args) => {
   const { scope } = npm.flatOptions
+  const registry = getRegistry(npm.flatOptions)
+  const auth = getAuthType(npm.flatOptions)
   const creds = npm.config.getCredentialsByURI(registry)
 
   log.disableProgress()
 
-  const auth = getAuthType(npm.flatOptions)
+  const { message, newCreds } = await auth({
+    creds,
+    registry,
+    scope
+  })
 
-  // XXX make auth.login() promise-returning so we don't have to wrap here
-  await new Promise((res, rej) => {
-    auth.login(creds, registry, scope, function (er, newCreds) {
-      if (er) {
-        return rej(er)
-      }
-
-      npm.config.del('_token', 'user') // prevent legacy pollution
-      if (scope) {
-        npm.config.set(scope + ':registry', registry, 'user')
-      }
-      npm.config.setCredentialsByURI(registry, newCreds)
-      npm.config.save('user', er => er ? rej(er) : res())
-    })
+  await updateConfig({
+    newCreds,
+    registry,
+    scope
   })
+
+  output(message)
 }
 
 module.exports = Object.assign(cmd, { completion, usage })

lib/auth/legacy.js

@@ -1,75 +1,101 @@
 'use strict'
 
-const read = require('../utils/read-user-info.js')
-const profile = require('npm-profile')
 const log = require('npmlog')
-const npm = require('../npm.js')
-const output = require('../utils/output.js')
-const openUrl = require('../utils/open-url')
+const profile = require('npm-profile')
 
+const openUrl = require('../utils/open-url.js')
+const read = require('../utils/read-user-info.js')
+
+// TODO: refactor lib/utils/open-url and its usages
 const openerPromise = (url) => new Promise((resolve, reject) => {
   openUrl(url, 'to complete your login please visit', (er) => er ? reject(er) : resolve())
 })
 
-const loginPrompter = (creds) => {
+const loginPrompter = async (creds) => {
   const opts = { log: log }
-  return read.username('Username:', creds.username, opts).then((u) => {
-    creds.username = u
-    return read.password('Password:', creds.password)
-  }).then((p) => {
-    creds.password = p
-    return read.email('Email: (this IS public) ', creds.email, opts)
-  }).then((e) => {
-    creds.email = e
-    return creds
-  })
+
+  creds.username = await read.username('Username:', creds.username, opts)
+  creds.password = await read.password('Password:', creds.password)
+  creds.email = await read.email('Email: (this IS public) ', creds.email, opts)
+
+  return creds
 }
 
-module.exports.login = (creds = {}, registry, scope, cb) => {
-  const opts = {
-    ...npm.flatOptions,
-    scope,
-    registry,
-    creds
+const login = async (opts) => {
+  let res
+
+  const requestOTP = async () => {
+    const otp = await read.otp(
+      'Enter one-time password from your authenticator app: '
+    )
+
+    return profile.loginCouch(
+      opts.creds.username,
+      opts.creds.password,
+      { ...opts, otp }
+    )
   }
-  login(opts).then((newCreds) => cb(null, newCreds)).catch(cb)
-}
 
-function login (opts) {
-  return profile.login(openerPromise, loginPrompter, opts)
-    .catch((err) => {
-      if (err.code === 'EOTP') throw err
-      const u = opts.creds.username
-      const p = opts.creds.password
-      const e = opts.creds.email
-      if (!(u && p && e)) throw err
-      return profile.adduserCouch(u, e, p, opts)
-    })
-    .catch((err) => {
-      if (err.code !== 'EOTP') throw err
-      return read.otp(
-        'Enter one-time password from your authenticator app: '
-      ).then(otp => {
-        const u = opts.creds.username
-        const p = opts.creds.password
-        return profile.loginCouch(u, p, { ...opts, otp })
-      })
-    }).then((result) => {
-      const newCreds = {}
-      if (result && result.token) {
-        newCreds.token = result.token
+  const addNewUser = async () => {
+    let newUser
+
+    try {
+      newUser = await profile.adduserCouch(
+        opts.creds.username,
+        opts.creds.email,
+        opts.creds.password,
+        opts
+      )
+    } catch (err) {
+      if (err.code === 'EOTP') {
+        newUser = await requestOTP()
       } else {
-        newCreds.username = opts.creds.username
-        newCreds.password = opts.creds.password
-        newCreds.email = opts.creds.email
-        newCreds.alwaysAuth = opts.alwaysAuth
+        throw err
       }
+    }
 
-      const usermsg = opts.creds.username ? ' user ' + opts.creds.username : ''
-      opts.log.info('login', 'Authorized' + usermsg)
-      const scopeMessage = opts.scope ? ' to scope ' + opts.scope : ''
-      const userout = opts.creds.username ? ' as ' + opts.creds.username : ''
-      output('Logged in%s%s on %s.', userout, scopeMessage, opts.registry)
-      return newCreds
-    })
+    return newUser
+  }
+
+  try {
+    res = await profile.login(openerPromise, loginPrompter, opts)
+  } catch (err) {
+    const needsMoreInfo = !(opts &&
+      opts.creds &&
+      opts.creds.username &&
+      opts.creds.password &&
+      opts.creds.email)
+    if (err.code === 'EOTP') {
+      res = await requestOTP()
+    } else if (needsMoreInfo) {
+      throw err
+    } else {
+      // TODO: maybe this needs to check for err.code === 'E400' instead?
+      res = await addNewUser()
+    }
+  }
+
+  const newCreds = {}
+  if (res && res.token) {
+    newCreds.token = res.token
+  } else {
+    newCreds.username = opts.creds.username
+    newCreds.password = opts.creds.password
+    newCreds.email = opts.creds.email
+    newCreds.alwaysAuth = opts.creds.alwaysAuth
+  }
+
+  const usermsg = opts.creds.username ? ` user ${opts.creds.username}` : ''
+  const scopeMessage = opts.scope ? ` to scope ${opts.scope}` : ''
+  const userout = opts.creds.username ? ` as ${opts.creds.username}` : ''
+  const message = `Logged in${userout}${scopeMessage} on ${opts.registry}.`
+
+  log.info('login', `Authorized${usermsg}`)
+
+  return {
+    message,
+    newCreds
+  }
 }
+
+module.exports = login

lib/auth/oauth.js

@@ -1,7 +1,9 @@
-var ssoAuth = require('./sso')
-var npm = require('../npm')
+const sso = require('./sso.js')
+const npm = require('../npm.js')
 
-module.exports.login = function login () {
+const login = (opts) => {
   npm.config.set('sso-type', 'oauth')
-  ssoAuth.login.apply(this, arguments)
+  return sso(opts)
 }
+
+module.exports = login

lib/auth/saml.js

@@ -1,7 +1,9 @@
-var ssoAuth = require('./sso')
-var npm = require('../npm')
+const sso = require('./sso.js')
+const npm = require('../npm.js')
 
-module.exports.login = function login () {
+const login = (opts) => {
   npm.config.set('sso-type', 'saml')
-  ssoAuth.login.apply(this, arguments)
+  return sso(opts)
 }
+
+module.exports = login

lib/auth/sso.js

@@ -9,47 +9,17 @@
 // CLI, we can remove this, and fold the lib/auth/legacy.js back into
 // lib/adduser.js
 
+const { promisify } = require('util')
+
 const log = require('npmlog')
-const npm = require('../npm.js')
+const profile = require('npm-profile')
 const npmFetch = require('npm-registry-fetch')
-const output = require('../utils/output.js')
-const { promisify } = require('util')
+
+const npm = require('../npm.js')
 const openUrl = promisify(require('../utils/open-url.js'))
 const otplease = require('../utils/otplease.js')
-const profile = require('npm-profile')
-
-module.exports.login = function login (creds, registry, scope, cb) {
-  log.warn('deprecated', 'SSO --auth-type is deprecated')
-  const opts = { ...npm.flatOptions, creds, registry, scope }
-  const ssoType = opts.ssoType
-  if (!ssoType) { return cb(new Error('Missing option: sso-type')) }
 
-  // We're reusing the legacy login endpoint, so we need some dummy
-  // stuff here to pass validation. They're never used.
-  const auth = {
-    username: 'npm_' + ssoType + '_auth_dummy_user',
-    password: 'placeholder',
-    email: 'support@npmjs.com',
-    authType: ssoType
-  }
-
-  otplease(opts,
-    opts => profile.loginCouch(auth.username, auth.password, opts)
-  ).then(({ token, sso }) => {
-    if (!token) { throw new Error('no SSO token returned') }
-    if (!sso) { throw new Error('no SSO URL returned by services') }
-    return openUrl(sso, 'to complete your login please visit').then(() => {
-      return pollForSession(registry, token, opts)
-    }).then(username => {
-      log.info('adduser', 'Authorized user %s', username)
-      var scopeMessage = scope ? ' to scope ' + scope : ''
-      output('Logged in as %s%s on %s.', username, scopeMessage, registry)
-      return { token }
-    })
-  }).then(res => cb(null, res), cb)
-}
-
-function pollForSession (registry, token, opts) {
+const pollForSession = ({ registry, token, opts }) => {
   log.info('adduser', 'Polling for validated SSO session')
   return npmFetch.json(
     '/-/whoami', { ...opts, registry, forceAuth: { token } }
@@ -58,7 +28,7 @@ function pollForSession (registry, token, opts) {
     err => {
       if (err.code === 'E401') {
         return sleep(opts.ssoPollFrequency).then(() => {
-          return pollForSession(registry, token, opts)
+          return pollForSession({ registry, token, opts })
         })
       } else {
         throw err
@@ -70,3 +40,46 @@ function pollForSession (registry, token, opts) {
 function sleep (time) {
   return new Promise((resolve) => setTimeout(resolve, time))
 }
+
+const login = async ({ creds, registry, scope }) => {
+  log.warn('deprecated', 'SSO --auth-type is deprecated')
+
+  const opts = { ...npm.flatOptions, creds, registry, scope }
+  const { ssoType } = opts
+
+  if (!ssoType) {
+    throw new Error('Missing option: sso-type')
+  }
+
+  // We're reusing the legacy login endpoint, so we need some dummy
+  // stuff here to pass validation. They're never used.
+  const auth = {
+    username: 'npm_' + ssoType + '_auth_dummy_user',
+    password: 'placeholder',
+    email: 'support@npmjs.com',
+    authType: ssoType
+  }
+
+  const { token, sso } = await otplease(opts,
+    opts => profile.loginCouch(auth.username, auth.password, opts)
+  )
+
+  if (!token) { throw new Error('no SSO token returned') }
+  if (!sso) { throw new Error('no SSO URL returned by services') }
+
+  await openUrl(sso, 'to complete your login please visit')
+
+  const username = await pollForSession({ registry, token, opts })
+
+  log.info('adduser', `Authorized user ${username}`)
+
+  const scopeMessage = scope ? ' to scope ' + scope : ''
+  const message = `Logged in as ${username}${scopeMessage} on ${registry}.`
+
+  return {
+    message,
+    newCreds: { token }
+  }
+}
+
+module.exports = login