Skip to content

fix: add otplease for enable-2fa, disable-2fa, access #8228

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

Open
wants to merge 9 commits into
base: latest
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/commands/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,15 @@ class Access extends BaseCommand {
}

async #grant (permissions, scope, pkg) {
await libnpmaccess.setPermissions(scope, pkg, permissions, this.npm.flatOptions)
await otplease(this.npm, this.npm.flatOptions, async (opts) => {
await libnpmaccess.setPermissions(scope, pkg, permissions, opts)
})
}

async #revoke (scope, pkg) {
await libnpmaccess.removePermissions(scope, pkg, this.npm.flatOptions)
await otplease(this.npm, this.npm.flatOptions, async (opts) => {
await libnpmaccess.removePermissions(scope, pkg, opts)
})
}

async #listPackages (owner, pkg) {
Expand Down
37 changes: 15 additions & 22 deletions lib/commands/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ class Profile extends BaseCommand {
}

async enable2fa (args) {
const conf = { ...this.npm.flatOptions }

if (args.length > 1) {
throw new Error('npm profile enable-2fa [auth-and-writes|auth-only]')
}
Expand All @@ -244,9 +246,16 @@ class Profile extends BaseCommand {
)
}

const userInfo = await get(conf)

if (!userInfo?.tfa?.pending && userInfo?.tfa?.mode === mode) {
output.standard('Two factor authentication is already enabled and set to ' + mode)
return
}

const info = {
tfa: {
mode: mode,
mode,
},
}

Expand Down Expand Up @@ -296,25 +305,15 @@ class Profile extends BaseCommand {
const password = await readUserInfo.password()
info.tfa.password = password

log.info('profile', 'Determine if tfa is pending')
const userInfo = await get({ ...this.npm.flatOptions })

const conf = { ...this.npm.flatOptions }
if (userInfo && userInfo.tfa && userInfo.tfa.pending) {
log.info('profile', 'Resetting two-factor authentication')
await set({ tfa: { password, mode: 'disable' } }, conf)
} else if (userInfo && userInfo.tfa) {
if (!conf.otp) {
conf.otp = await readUserInfo.otp(
'Enter one-time password: '
)
}
}

log.info('profile', 'Setting two-factor authentication to ' + mode)
const challenge = await set(info, conf)
const challenge = await otplease(this.npm, conf, c => set(info, c))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const challenge = await otplease(this.npm, conf, c => set(info, c))
const challenge = await otplease(this.npm, conf, o => set(info, o))

I know this is nitpicky but I believe we have tried to consistently refer to this param as "opts", shortened to "o" sometimes.


if (challenge.tfa === null) {
if (challenge.tfa && challenge.tfa.mode) {
output.standard('Two factor authentication mode changed to: ' + mode)
return
}
Expand Down Expand Up @@ -358,8 +357,8 @@ class Profile extends BaseCommand {
}

async disable2fa () {
const conf = { ...this.npm.flatOptions }
const info = await get(conf)
const opts = { ...this.npm.flatOptions }
const info = await get(opts)

if (!info.tfa || info.tfa.pending) {
output.standard('Two factor authentication not enabled.')
Expand All @@ -368,14 +367,8 @@ class Profile extends BaseCommand {

const password = await readUserInfo.password()

if (!conf.otp) {
const msg = 'Enter one-time password: '
conf.otp = await readUserInfo.otp(msg)
}

log.info('profile', 'disabling tfa')

await set({ tfa: { password: password, mode: 'disable' } }, conf)
await otplease(this.npm, opts, o => set({ tfa: { password: password, mode: 'disable' } }, o))

if (this.npm.config.get('json')) {
output.buffer({ tfa: false })
Expand Down
Loading
Loading