From 65a09b003fe5e4f71ba3de509191b143d7b24eb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20M=C3=B8ller=20Ellehauge?= Date: Mon, 27 Jun 2022 11:08:06 +0200 Subject: [PATCH 01/10] refactor: change flow of otplease --- lib/utils/otplease.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/utils/otplease.js b/lib/utils/otplease.js index 83985b6bc170d..d8db8add9c4cc 100644 --- a/lib/utils/otplease.js +++ b/lib/utils/otplease.js @@ -2,15 +2,20 @@ async function otplease (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 { + } + + const isBackwardsCompatibleOTP = err.code === 'E401' && /one-time pass/.test(err.body) + const isClassicOTP = err.code === 'EOTP' + + if (isClassicOTP || isBackwardsCompatibleOTP) { + 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 } } From 87b9a51ae6b0fda23f5aa117659717e4031a7605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20M=C3=B8ller=20Ellehauge?= Date: Wed, 29 Jun 2022 11:57:28 +0200 Subject: [PATCH 02/10] refactor: add npm argument to otplease --- lib/auth/sso.js | 2 +- lib/commands/access.js | 2 +- lib/commands/deprecate.js | 2 +- lib/commands/dist-tag.js | 4 ++-- lib/commands/hook.js | 2 +- lib/commands/org.js | 2 +- lib/commands/owner.js | 2 +- lib/commands/profile.js | 2 +- lib/commands/publish.js | 2 +- lib/commands/team.js | 2 +- lib/commands/token.js | 4 ++-- lib/commands/unpublish.js | 2 +- lib/utils/otplease.js | 2 +- test/lib/utils/otplease.js | 20 ++++++++++++++------ 14 files changed, 29 insertions(+), 21 deletions(-) diff --git a/lib/auth/sso.js b/lib/auth/sso.js index 9812a18cb99ca..621ead5d21b65 100644 --- 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 index bc8ce48bacdad..0a80da8ddd006 100644 --- 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 index 862c214dbe592..068bfdbcec717 100644 --- 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 index e74a3f1d435c9..8052e4f7e4e38 100644 --- 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 index a4619802d8429..bb3a34b8d2d1b 100644 --- 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 index e2202a9e9cf3b..599b4b9c8758a 100644 --- 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 index 732bb40a30050..824b64e044ecf 100644 --- 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 index fcf0eb7d53fa6..27060cf73a650 100644 --- 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 index 579f5d6e74e67..4310dba258d95 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -116,7 +116,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 index 640002456acad..2d4fc663715e4 100644 --- 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 index edfb07b9d3a9a..cf3b8cbee53a4 100644 --- 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 index ab929d98cadfa..0e5ef3dc5e91d 100644 --- 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 index d8db8add9c4cc..790f74bd9704f 100644 --- a/lib/utils/otplease.js +++ b/lib/utils/otplease.js @@ -1,4 +1,4 @@ -async function otplease (opts, fn) { +async function otplease (npm, opts, fn) { try { return await fn(opts) } catch (err) { diff --git a/test/lib/utils/otplease.js b/test/lib/utils/otplease.js index 025084ab4f2c5..de85fc6c5914e 100644 --- a/test/lib/utils/otplease.js +++ b/test/lib/utils/otplease.js @@ -11,7 +11,7 @@ const otplease = t.mock('../../../lib/utils/otplease.js', { 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 +26,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 +51,7 @@ t.test('prompts for otp for EOTP', async (t) => { t.end() } - await otplease({ some: 'prop' }, fn) + await otplease(null, { some: 'prop' }, fn) }) t.test('prompts for otp for 401', async (t) => { @@ -78,7 +78,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 +95,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 +116,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' + ) }) From 51adcc056aefc6c73eade1ea7bd1c80dc50a9e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20M=C3=B8ller=20Ellehauge?= Date: Wed, 29 Jun 2022 12:31:12 +0200 Subject: [PATCH 03/10] feat: Add support for web auth, utilizing code from npm-profile --- lib/utils/otplease.js | 17 +++++++++++++++++ lib/utils/web-auth.js | 22 ++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 lib/utils/web-auth.js diff --git a/lib/utils/otplease.js b/lib/utils/otplease.js index 790f74bd9704f..8bfe7da1d387f 100644 --- a/lib/utils/otplease.js +++ b/lib/utils/otplease.js @@ -8,6 +8,23 @@ async function otplease (npm, opts, fn) { const isBackwardsCompatibleOTP = err.code === 'E401' && /one-time pass/.test(err.body) const isClassicOTP = err.code === 'EOTP' + const isWebOTP = err.code === 'EOTP' && err.body?.authUrl && err.body?.doneUrl + + if (isWebOTP) { + 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 || isBackwardsCompatibleOTP) { const readUserInfo = require('./read-user-info.js') diff --git a/lib/utils/web-auth.js b/lib/utils/web-auth.js new file mode 100644 index 0000000000000..1a7e227de2191 --- /dev/null +++ b/lib/utils/web-auth.js @@ -0,0 +1,22 @@ +const EventEmitter = require('events') +const { webAuthCheckLogin } = require('npm-profile') + +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 + }) + + return Promise.all([openPromise, webAuthCheckPromise]).then( + // pick the auth result and pass it along + ([, authResult]) => authResult + ) +} + +module.exports = webAuth From ac30d4c9a3191d0f1fc5c491ee9bcd41ce6061de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20M=C3=B8ller=20Ellehauge?= Date: Wed, 29 Jun 2022 12:45:29 +0200 Subject: [PATCH 04/10] refactor: simplify boolean variables --- lib/utils/otplease.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/utils/otplease.js b/lib/utils/otplease.js index 8bfe7da1d387f..44f9aaa89b7a3 100644 --- a/lib/utils/otplease.js +++ b/lib/utils/otplease.js @@ -6,8 +6,8 @@ async function otplease (npm, opts, fn) { throw err } - const isBackwardsCompatibleOTP = err.code === 'E401' && /one-time pass/.test(err.body) - const isClassicOTP = err.code === 'EOTP' + const isClassicOTP = + err.code === 'EOTP' || (err.code === 'E401' && /one-time pass/.test(err.body)) const isWebOTP = err.code === 'EOTP' && err.body?.authUrl && err.body?.doneUrl if (isWebOTP) { @@ -26,7 +26,7 @@ async function otplease (npm, opts, fn) { return await fn({ ...opts, otp }) } - if (isClassicOTP || isBackwardsCompatibleOTP) { + if (isClassicOTP) { 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 }) From ed08d7cc637bbb617745c6ca0c568093cc9c31ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20M=C3=B8ller=20Ellehauge?= Date: Fri, 1 Jul 2022 10:49:00 +0200 Subject: [PATCH 05/10] Make `webAuth` async Co-authored-by: Jordan Harband --- lib/utils/web-auth.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/web-auth.js b/lib/utils/web-auth.js index 1a7e227de2191..5c291fabb5fa9 100644 --- a/lib/utils/web-auth.js +++ b/lib/utils/web-auth.js @@ -1,7 +1,7 @@ const EventEmitter = require('events') const { webAuthCheckLogin } = require('npm-profile') -function webAuth (opener, initialUrl, doneUrl, opts) { +async function webAuth (opener, initialUrl, doneUrl, opts) { const doneEmitter = new EventEmitter() const openPromise = opener(initialUrl, doneEmitter) From 648cb04c5e9bfb8c59a0418d60e2b9b511d48150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20M=C3=B8ller=20Ellehauge?= Date: Fri, 1 Jul 2022 10:51:02 +0200 Subject: [PATCH 06/10] Remove 'Promise.all' --- lib/utils/web-auth.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/utils/web-auth.js b/lib/utils/web-auth.js index 5c291fabb5fa9..b995998809c43 100644 --- a/lib/utils/web-auth.js +++ b/lib/utils/web-auth.js @@ -13,10 +13,8 @@ async function webAuth (opener, initialUrl, doneUrl, opts) { return authResult }) - return Promise.all([openPromise, webAuthCheckPromise]).then( - // pick the auth result and pass it along - ([, authResult]) => authResult - ) + await openPromise + return await webAuthCheckPromise } module.exports = webAuth From a0fd462b562d86da76bce8d8d79cdf5d93f6922a Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 13 Jul 2022 16:05:09 +0100 Subject: [PATCH 07/10] test webauth in otplease --- test/lib/utils/otplease.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/lib/utils/otplease.js b/test/lib/utils/otplease.js index de85fc6c5914e..79eaa798e6053 100644 --- a/test/lib/utils/otplease.js +++ b/test/lib/utils/otplease.js @@ -1,12 +1,20 @@ 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) => { @@ -54,6 +62,30 @@ t.test('prompts for otp for EOTP', async (t) => { 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) => { const stdinTTY = process.stdin.isTTY const stdoutTTY = process.stdout.isTTY From 2a4c69041795033ebb5c2c9d7f8ed01714aa4fb1 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 13 Jul 2022 16:22:44 +0100 Subject: [PATCH 08/10] remove conditional chaining operator --- lib/utils/otplease.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/utils/otplease.js b/lib/utils/otplease.js index 44f9aaa89b7a3..0e20e7a797ae0 100644 --- a/lib/utils/otplease.js +++ b/lib/utils/otplease.js @@ -6,11 +6,7 @@ async function otplease (npm, opts, fn) { throw err } - const isClassicOTP = - err.code === 'EOTP' || (err.code === 'E401' && /one-time pass/.test(err.body)) - const isWebOTP = err.code === 'EOTP' && err.body?.authUrl && err.body?.doneUrl - - if (isWebOTP) { + if (isWebOTP(err)) { const webAuth = require('./web-auth') const openUrlPrompt = require('./open-url-prompt') @@ -26,7 +22,7 @@ async function otplease (npm, opts, fn) { return await fn({ ...opts, otp }) } - if (isClassicOTP) { + 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 }) @@ -36,4 +32,15 @@ async function otplease (npm, opts, fn) { } } +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 From 38cef3ab44d3fbaa5f45e56b02eb37b50d62083c Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Wed, 13 Jul 2022 16:42:12 +0100 Subject: [PATCH 09/10] add tests for web-auth --- lib/utils/web-auth.js | 2 +- test/lib/utils/web-auth.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 test/lib/utils/web-auth.js diff --git a/lib/utils/web-auth.js b/lib/utils/web-auth.js index b995998809c43..ce551687098fc 100644 --- a/lib/utils/web-auth.js +++ b/lib/utils/web-auth.js @@ -10,7 +10,7 @@ async function webAuth (opener, initialUrl, doneUrl, opts) { // cancel open prompt if it's present doneEmitter.emit('abort') - return authResult + return authResult.token }) await openPromise diff --git a/test/lib/utils/web-auth.js b/test/lib/utils/web-auth.js new file mode 100644 index 0000000000000..ee8a17ecbc09d --- /dev/null +++ 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') +}) From 1231bf7d614f732414a3774f1e17e264f9b7499f Mon Sep 17 00:00:00 2001 From: Sandeep Meduru Date: Tue, 19 Jul 2022 23:08:34 +0530 Subject: [PATCH 10/10] Disable progress bar on publish --- lib/commands/publish.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 4310dba258d95..0b6eed04b009b 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -62,6 +62,7 @@ class Publish extends BaseCommand { } const opts = { ...this.npm.flatOptions } + log.disableProgress() // you can publish name@version, ./foo.tgz, etc. // even though the default is the 'file:.' cwd.