Skip to content

Commit c8bdb4a

Browse files
jumoelljharbhfauldssandeepmeduru
authored andcommitted
feat: Support pure web authentication for commands
* feat: Add support for web auth, utilizing code from npm-profile Co-authored-by: Jordan Harband <[email protected]> Co-authored-by: Hayden Faulds <[email protected]> Co-authored-by: Sandeep Meduru <[email protected]>
1 parent 68ade72 commit c8bdb4a

File tree

16 files changed

+148
-26
lines changed

16 files changed

+148
-26
lines changed

lib/auth/sso.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const login = async (npm, { creds, registry, scope }) => {
5252
authType: ssoType,
5353
}
5454

55-
const { token, sso } = await otplease(opts,
55+
const { token, sso } = await otplease(npm, opts,
5656
opts => profile.loginCouch(auth.username, auth.password, opts)
5757
)
5858

lib/commands/access.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ class Access extends BaseCommand {
180180

181181
modifyPackage (pkg, opts, fn, requireScope = true) {
182182
return this.getPackage(pkg, requireScope)
183-
.then(pkgName => otplease(opts, opts => fn(pkgName, opts)))
183+
.then(pkgName => otplease(this.npm, opts, opts => fn(pkgName, opts)))
184184
}
185185

186186
async getPackage (name, requireScope) {

lib/commands/deprecate.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class Deprecate extends BaseCommand {
6060
packument.versions[v].deprecated = msg
6161
})
6262

63-
return otplease(this.npm.flatOptions, opts => fetch(uri, {
63+
return otplease(this.npm, this.npm.flatOptions, opts => fetch(uri, {
6464
...opts,
6565
spec: p,
6666
method: 'PUT',

lib/commands/dist-tag.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ class DistTag extends BaseCommand {
116116
},
117117
spec,
118118
}
119-
await otplease(reqOpts, reqOpts => regFetch(url, reqOpts))
119+
await otplease(this.npm, reqOpts, reqOpts => regFetch(url, reqOpts))
120120
this.npm.output(`+${t}: ${spec.name}@${version}`)
121121
}
122122

@@ -142,7 +142,7 @@ class DistTag extends BaseCommand {
142142
method: 'DELETE',
143143
spec,
144144
}
145-
await otplease(reqOpts, reqOpts => regFetch(url, reqOpts))
145+
await otplease(this.npm, reqOpts, reqOpts => regFetch(url, reqOpts))
146146
this.npm.output(`-${tag}: ${spec.name}@${version}`)
147147
}
148148

lib/commands/hook.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class Hook extends BaseCommand {
2222
static ignoreImplicitWorkspace = true
2323

2424
async exec (args) {
25-
return otplease({
25+
return otplease(this.npm, {
2626
...this.npm.flatOptions,
2727
}, (opts) => {
2828
switch (args[0]) {

lib/commands/org.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class Org extends BaseCommand {
3333
}
3434

3535
async exec ([cmd, orgname, username, role], cb) {
36-
return otplease({
36+
return otplease(this.npm, {
3737
...this.npm.flatOptions,
3838
}, opts => {
3939
switch (cmd) {

lib/commands/owner.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ class Owner extends BaseCommand {
202202

203203
const dataPath = `/${spec.escapedName}/-rev/${encodeURIComponent(data._rev)}`
204204
try {
205-
const res = await otplease(this.npm.flatOptions, opts => {
205+
const res = await otplease(this.npm, this.npm.flatOptions, opts => {
206206
return npmFetch.json(dataPath, {
207207
...opts,
208208
method: 'PUT',

lib/commands/profile.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ class Profile extends BaseCommand {
221221

222222
newUser[prop] = value
223223

224-
const result = await otplease(conf, conf => npmProfile.set(newUser, conf))
224+
const result = await otplease(this.npm, conf, conf => npmProfile.set(newUser, conf))
225225

226226
if (this.npm.config.get('json')) {
227227
this.npm.output(JSON.stringify({ [prop]: result[prop] }, null, 2))

lib/commands/publish.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ class Publish extends BaseCommand {
6262
}
6363

6464
const opts = { ...this.npm.flatOptions, progress: false }
65+
log.disableProgress()
6566

6667
// you can publish name@version, ./foo.tgz, etc.
6768
// even though the default is the 'file:.' cwd.
@@ -116,7 +117,7 @@ class Publish extends BaseCommand {
116117
log.notice('', `Publishing to ${outputRegistry}${dryRun ? ' (dry-run)' : ''}`)
117118

118119
if (!dryRun) {
119-
await otplease(opts, opts => libpub(manifest, tarballData, opts))
120+
await otplease(this.npm, opts, opts => libpub(manifest, tarballData, opts))
120121
}
121122

122123
if (spec.type === 'directory' && !ignoreScripts) {

lib/commands/team.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class Team extends BaseCommand {
4444
// XXX: "description" option to libnpmteam is used as a description of the
4545
// team, but in npm's options, this is a boolean meaning "show the
4646
// description in npm search output". Hence its being set to null here.
47-
await otplease({ ...this.npm.flatOptions }, opts => {
47+
await otplease(this.npm, { ...this.npm.flatOptions }, opts => {
4848
entity = entity.replace(/^@/, '')
4949
switch (cmd) {
5050
case 'create': return this.create(entity, opts)

lib/commands/token.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ class Token extends BaseCommand {
121121
})
122122
await Promise.all(
123123
toRemove.map(key => {
124-
return otplease(conf, conf => {
124+
return otplease(this.npm, conf, conf => {
125125
return profile.removeToken(key, conf)
126126
})
127127
})
@@ -146,7 +146,7 @@ class Token extends BaseCommand {
146146
const validCIDR = this.validateCIDRList(cidr)
147147
log.info('token', 'creating')
148148
return pulseTillDone.withPromise(
149-
otplease(conf, conf => {
149+
otplease(this.npm, conf, conf => {
150150
return profile.createToken(password, readonly, validCIDR, conf)
151151
})
152152
)

lib/commands/unpublish.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ class Unpublish extends BaseCommand {
130130
}
131131

132132
if (!dryRun) {
133-
await otplease(opts, opts => libunpub(spec, opts))
133+
await otplease(this.npm, opts, opts => libunpub(spec, opts))
134134
}
135135
if (!silent) {
136136
this.npm.output(`- ${pkgName}${pkgVersion}`)

lib/utils/otplease.js

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,46 @@
1-
async function otplease (opts, fn) {
1+
async function otplease (npm, opts, fn) {
22
try {
33
return await fn(opts)
44
} catch (err) {
5-
const readUserInfo = require('./read-user-info.js')
6-
if (err.code !== 'EOTP' && (err.code !== 'E401' || !/one-time pass/.test(err.body))) {
5+
if (!process.stdin.isTTY || !process.stdout.isTTY) {
76
throw err
8-
} else if (!process.stdin.isTTY || !process.stdout.isTTY) {
9-
throw err
10-
} else {
7+
}
8+
9+
if (isWebOTP(err)) {
10+
const webAuth = require('./web-auth')
11+
const openUrlPrompt = require('./open-url-prompt')
12+
13+
const openerPromise = (url, emitter) =>
14+
openUrlPrompt(
15+
npm,
16+
url,
17+
'Authenticate your account at',
18+
'Press ENTER to open in the browser...',
19+
emitter
20+
)
21+
const otp = await webAuth(openerPromise, err.body.authUrl, err.body.doneUrl, opts)
22+
return await fn({ ...opts, otp })
23+
}
24+
25+
if (isClassicOTP(err)) {
26+
const readUserInfo = require('./read-user-info.js')
1127
const otp = await readUserInfo.otp('This operation requires a one-time password.\nEnter OTP:')
1228
return await fn({ ...opts, otp })
1329
}
30+
31+
throw err
32+
}
33+
}
34+
35+
function isWebOTP (err) {
36+
if (!err.code === 'EOTP' || !err.body) {
37+
return false
1438
}
39+
return err.body.authUrl && err.body.doneUrl
40+
}
41+
42+
function isClassicOTP (err) {
43+
return err.code === 'EOTP' || (err.code === 'E401' && /one-time pass/.test(err.body))
1544
}
1645

1746
module.exports = otplease

lib/utils/web-auth.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
const EventEmitter = require('events')
2+
const { webAuthCheckLogin } = require('npm-profile')
3+
4+
async function webAuth (opener, initialUrl, doneUrl, opts) {
5+
const doneEmitter = new EventEmitter()
6+
7+
const openPromise = opener(initialUrl, doneEmitter)
8+
const webAuthCheckPromise = webAuthCheckLogin(doneUrl, { ...opts, cache: false })
9+
.then(authResult => {
10+
// cancel open prompt if it's present
11+
doneEmitter.emit('abort')
12+
13+
return authResult.token
14+
})
15+
16+
await openPromise
17+
return await webAuthCheckPromise
18+
}
19+
20+
module.exports = webAuth

test/lib/utils/otplease.js

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,25 @@
11
const t = require('tap')
2+
3+
const { fake: mockNpm } = require('../../fixtures/mock-npm')
24
const mockGlobals = require('../../fixtures/mock-globals')
35

46
const readUserInfo = {
57
otp: async () => '1234',
68
}
9+
const webAuth = async (opener) => {
10+
opener()
11+
return '1234'
12+
}
713

814
const otplease = t.mock('../../../lib/utils/otplease.js', {
915
'../../../lib/utils/read-user-info.js': readUserInfo,
16+
'../../../lib/utils/open-url-prompt.js': () => {},
17+
'../../../lib/utils/web-auth': webAuth,
1018
})
1119

1220
t.test('returns function results on success', async (t) => {
1321
const fn = () => 'test string'
14-
const result = await otplease({}, fn)
22+
const result = await otplease(null, {}, fn)
1523
t.equal('test string', result)
1624
})
1725

@@ -26,7 +34,7 @@ t.test('returns function results on otp success', async (t) => {
2634
}
2735
throw Object.assign(new Error('nope'), { code: 'EOTP' })
2836
}
29-
const result = await otplease({}, fn)
37+
const result = await otplease(null, {}, fn)
3038
t.equal('success', result)
3139
})
3240

@@ -51,7 +59,31 @@ t.test('prompts for otp for EOTP', async (t) => {
5159
t.end()
5260
}
5361

54-
await otplease({ some: 'prop' }, fn)
62+
await otplease(null, { some: 'prop' }, fn)
63+
})
64+
65+
t.test('returns function results on webauth success', async (t) => {
66+
mockGlobals(t, {
67+
'process.stdin': { isTTY: true },
68+
'process.stdout': { isTTY: true },
69+
})
70+
71+
const npm = mockNpm({ config: { browser: 'firefox' } })
72+
const fn = ({ otp }) => {
73+
if (otp) {
74+
return 'success'
75+
}
76+
throw Object.assign(new Error('nope'), {
77+
code: 'EOTP',
78+
body: {
79+
authUrl: 'https://www.example.com/auth',
80+
doneUrl: 'https://www.example.com/done',
81+
},
82+
})
83+
}
84+
85+
const result = await otplease(npm, {}, fn)
86+
t.equal('success', result)
5587
})
5688

5789
t.test('prompts for otp for 401', async (t) => {
@@ -78,7 +110,7 @@ t.test('prompts for otp for 401', async (t) => {
78110
t.end()
79111
}
80112

81-
await otplease({ some: 'prop' }, fn)
113+
await otplease(null, { some: 'prop' }, fn)
82114
})
83115

84116
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) => {
95127
throw new Error('nope')
96128
}
97129

98-
t.rejects(otplease({ some: 'prop' }, fn), { message: 'nope' }, 'rejects with the original error')
130+
t.rejects(
131+
otplease(null, { some: 'prop' }, fn),
132+
{ message: 'nope' },
133+
'rejects with the original error'
134+
)
99135
})
100136

101137
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) => {
112148
throw Object.assign(new Error('nope'), { code: 'EOTP' })
113149
}
114150

115-
t.rejects(otplease({ some: 'prop' }, fn), { message: 'nope' }, 'rejects with the original error')
151+
t.rejects(
152+
otplease(null, { some: 'prop' }, fn),
153+
{ message: 'nope' },
154+
'rejects with the original error'
155+
)
116156
})

test/lib/utils/web-auth.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
const t = require('tap')
2+
3+
const webAuthCheckLogin = async () => {
4+
return { token: 'otp-token' }
5+
}
6+
7+
const webauth = t.mock('../../../lib/utils/web-auth.js', {
8+
'npm-profile': { webAuthCheckLogin },
9+
})
10+
11+
const initialUrl = 'https://example.com/auth'
12+
const doneUrl = 'https://example.com/done'
13+
const opts = {}
14+
15+
t.test('returns token on success', async (t) => {
16+
const opener = async () => {}
17+
const result = await webauth(opener, initialUrl, doneUrl, opts)
18+
t.equal(result, 'otp-token')
19+
})
20+
21+
t.test('closes opener when auth check finishes', async (t) => {
22+
const opener = (_url, emitter) => {
23+
return new Promise((resolve, reject) => {
24+
// the only way to finish this promise is to emit aboter on the emitter
25+
emitter.addListener('abort', () => {
26+
resolve()
27+
})
28+
})
29+
}
30+
const result = await webauth(opener, initialUrl, doneUrl, opts)
31+
t.equal(result, 'otp-token')
32+
})

0 commit comments

Comments
 (0)