Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support pure web authentication for commands #5098

Merged
merged 11 commits into from Jul 20, 2022
2 changes: 1 addition & 1 deletion lib/auth/sso.js
Expand Up @@ -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)
)

Expand Down
2 changes: 1 addition & 1 deletion lib/commands/access.js
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/deprecate.js
Expand Up @@ -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',
Expand Down
4 changes: 2 additions & 2 deletions lib/commands/dist-tag.js
Expand Up @@ -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}`)
}

Expand All @@ -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}`)
}

Expand Down
2 changes: 1 addition & 1 deletion lib/commands/hook.js
Expand Up @@ -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]) {
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/org.js
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/owner.js
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/profile.js
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/publish.js
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/team.js
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/commands/token.js
Expand Up @@ -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)
})
})
Expand All @@ -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)
})
)
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/unpublish.js
Expand Up @@ -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}`)
Expand Down
41 changes: 35 additions & 6 deletions 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
20 changes: 20 additions & 0 deletions 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
52 changes: 46 additions & 6 deletions 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)
})

Expand All @@ -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)
})

Expand All @@ -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) => {
Expand All @@ -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) => {
Expand All @@ -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) => {
Expand All @@ -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'
)
})
32 changes: 32 additions & 0 deletions 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')
})