fix: cancel opener promise if web login fails (#57)

Author: sandeepmeduru

lib/index.js

@@ -56,6 +56,7 @@ const webAuth = (opener, opts, body) => {
   const { hostname } = opts
   body.hostname = hostname || os.hostname()
   const target = '/-/v1/login'
+  const doneEmitter = new EventEmitter()
   return fetch(target, {
     ...opts,
     method: 'POST',
@@ -75,8 +76,6 @@ const webAuth = (opener, opts, body) => {
   }).then(({ doneUrl, loginUrl }) => {
     log.verbose('web auth', 'opening url pair')
 
-    const doneEmitter = new EventEmitter()
-
     const openPromise = opener(loginUrl, doneEmitter)
     const webAuthCheckPromise = webAuthCheckLogin(doneUrl, { ...opts, cache: false })
       .then(authResult => {
@@ -93,6 +92,9 @@ const webAuth = (opener, opts, body) => {
       ([, authResult]) => authResult
     )
   }).catch(er => {
+    // cancel open prompt if it's present
+    doneEmitter.emit('abort')
+
     if ((er.statusCode >= 400 && er.statusCode <= 499) || er.statusCode === 500) {
       throw new WebLoginNotSupported('POST', {
         status: er.statusCode,

test/index.js

@@ -16,7 +16,7 @@ test('get', t => {
     return [auth ? 200 : 401, '', {}]
   })
   return profile.get().then(result => {
-    t.fail('GET w/o auth should fail')
+    return t.fail('GET w/o auth should fail')
   }, err => {
     t.equal(err.code, 'E401', 'auth errors are passed through')
   }).then(() => {
@@ -27,7 +27,7 @@ test('get', t => {
     })
     return profile.get({ '//registry.npmjs.org/:_authToken': 'deadbeef' })
   }).then(result => {
-    t.match(result, { auth: 'bearer' })
+    return t.match(result, { auth: 'bearer' })
   }).then(() => {
     srv.get(getUrl).reply(function () {
       const auth = this.req.headers.authorization
@@ -46,7 +46,7 @@ test('get', t => {
       '//registry.npmjs.org/:_password': Buffer.from('123', 'utf8').toString('base64'),
     })
   }).then(result => {
-    t.match(result, { auth: 'basic' })
+    return t.match(result, { auth: 'basic' })
   }).then(() => {
     srv.get(getUrl).reply(function () {
       const auth = this.req.headers.authorization
@@ -60,7 +60,7 @@ test('get', t => {
       '//registry.npmjs.org/:_authToken': 'deadbeef',
     })
   }).then(result => {
-    t.match(result, { auth: 'bearer', otp: true })
+    return t.match(result, { auth: 'bearer', otp: true })
   })
   // with otp, with token, with basic
   // prob should make w/o token 401
@@ -76,7 +76,7 @@ test('set', t => {
     github: 'zkat',
     email: '',
   }).then(json => {
-    t.same(json, prof, 'got the profile data in return')
+    return t.same(json, prof, 'got the profile data in return')
   })
 })
 
@@ -125,6 +125,42 @@ test('login fallback to couch', t => {
   })
 })
 
+test('login fallback to couch when web login fails cancels opener promise', t => {
+  const loginUrl = 'https://www.npmjs.com/login?next=/login/cli/123'
+  tnock(t, registry)
+    .put('/-/user/org.couchdb.user:blerp')
+    .reply(201, {
+      ok: true,
+    })
+    .post('/-/v1/login')
+    .reply(200, {
+      loginUrl,
+      doneUrl: 'https://registry.npmjs.org:443/-/v1/done?sessionId=123',
+    })
+    .get('/-/v1/done?sessionId=123')
+    .reply(404, { error: 'Not found' })
+
+  let cancelled = false
+  const opener = (url, doneEmitter) => {
+    t.equal(url, loginUrl)
+    doneEmitter.on('abort', () => {
+      cancelled = true
+    })
+  }
+
+  const prompter = creds => Promise.resolve({
+    username: 'blerp',
+    password: 'prelb',
+    email: 'blerp@blerp.blerp',
+  })
+  return t.resolveMatch(profile.login(opener, prompter), {
+    ok: true,
+    username: 'blerp',
+  }).then(() => {
+    return t.equal(cancelled, true)
+  })
+})
+
 test('adduserCouch happy path', t => {
   tnock(t, registry)
     .put('/-/user/org.couchdb.user:blerp')
@@ -222,7 +258,7 @@ test('listTokens multipage', t => {
     urls: {},
   })
   return profile.listTokens().then(tok => {
-    t.same(
+    return t.same(
       tok,
       tokens1.concat(tokens2).concat(tokens3),
       'supports multi-URL token requests and concats them'
@@ -233,7 +269,7 @@ test('listTokens multipage', t => {
 test('removeToken', t => {
   tnock(t, registry).delete('/-/npm/v1/tokens/token/deadbeef').reply(200)
   return profile.removeToken('deadbeef').then(ret => {
-    t.equal(ret, null, 'null return value on success')
+    return t.equal(ret, null, 'null return value on success')
   })
 })