From c8bdb4a2517cf69cccf9bfe5bd5375a5e3d4b5b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20M=C3=B8ller=20Ellehauge?= Date: Wed, 20 Jul 2022 21:49:55 +0200 Subject: [PATCH] feat: Support pure web authentication for commands * feat: Add support for web auth, utilizing code from npm-profile Co-authored-by: Jordan Harband Co-authored-by: Hayden Faulds Co-authored-by: Sandeep Meduru --- 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 | 3 ++- lib/commands/team.js | 2 +- lib/commands/token.js | 4 +-- lib/commands/unpublish.js | 2 +- lib/utils/otplease.js | 41 +++++++++++++++++++++++++----- lib/utils/web-auth.js | 20 +++++++++++++++ test/lib/utils/otplease.js | 52 +++++++++++++++++++++++++++++++++----- test/lib/utils/web-auth.js | 32 +++++++++++++++++++++++ 16 files changed, 148 insertions(+), 26 deletions(-) create mode 100644 lib/utils/web-auth.js create mode 100644 test/lib/utils/web-auth.js 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 d61fff936c854..3d17866a684a4 100644 --- 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 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 83985b6bc170d..0e20e7a797ae0 100644 --- 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 new file mode 100644 index 0000000000000..ce551687098fc --- /dev/null +++ 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 index 025084ab4f2c5..79eaa798e6053 100644 --- 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 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') +})