Add OTP prompt during publish #2084

rbuckton · 2019-05-12T06:02:37Z

Description

Adds a prompt for a one-time-password (OTP) when requested from registries that use two-factor authentication (2FA).

Motivation and Context

It is currently marginally possible to publish using 2FA without this prompt, however this requires setting an environment variable prior to calling lerna publish, which could expire during the process of publishing packages.

How Has This Been Tested?

I've added tests for @lerna/otplease and have tested publishing a monorepo for an account that requires a one-time password.

Fixes: #1091

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

rbuckton · 2019-05-12T06:04:13Z

@evocateur: The only issue seems to be that the prompt gets lost in among other messages:

? Are you sure you want to publish these packages? Yes
lerna info execute Skipping GitHub releases
lerna info git Pushing tags...
lerna info publish Publishing packages to npm...
info Verifying npm credentials
http fetch GET 200 https://registry.npmjs.org/-/npm/v1/user 1911ms.
http fetch GET 200 https://registry.npmjs.org/-/org/rbuckton/package?format=cli 228ms
http fetch PUT 401 https://registry.npmjs.org/@esfx%2fdisposable 346mspat packages\reflect-metadata-compat
? This operation requires a one-time password.
http fetch PUT 401 https://registry.npmjs.org/@esfx%2finternal-murmur3 471msckages\reflect-metadata-compat
http fetch PUT 401 https://registry.npmjs.org/@esfx%2finternal-integers 1809ms
? This operation requires a one-time password.
Enter OTP: 123456

As I was typing the OTP the text kept getting replaced with the progress message from npmlog. I'm not familiar enough with @lerna/prompt or npmlog to know if there's anything I need to change to improve the experience.

rbuckton · 2019-05-12T06:26:14Z

I thought it might be the newline I added in the prompt, but I'm still seeing the issue after removing it:

? Are you sure you want to publish these packages? Yes
lerna info execute Skipping GitHub releases
lerna info git Pushing tags...
lerna info publish Publishing packages to npm...
info Verifying npm credentials
http fetch GET 200 https://registry.npmjs.org/-/npm/v1/user 1026ms.
http fetch GET 200 https://registry.npmjs.org/-/org/rbuckton/package?format=cli 164ms
http fetch PUT 401 https://registry.npmjs.org/@esfx%2finternal-integers 298mskages\reflect-metadata-compat
? This operation requires a one-time password: http fetch PUT 401 https://registry.npmjs.org/@esfx%2fcancelable 310ms
http fetch PUT 401 https://registry.npmjs.org/@esfx%2finternal-murmur3 1154mskages\reflect-metadata-compat
http fetch PUT 401 https://registry.npmjs.org/@esfx%2fdisposable 536mspat packages\reflect-metadata-compat
[     .............] - publish: verb packed @esfx/reflect-metadata-compat packages\reflect-metadata-compat

It seems like some of the messages sent to npmlog are being written even after log.pause() is called in prompt.input.

rbuckton · 2019-05-12T20:02:54Z

Ah, I think I figured out the reason. @lerna/publish and @lerna/npm-publish had duplicate installs of npmlog locally (non-deduped), so the log sent to libnpmpublish was a different instance and wasn't disabled.

Unfortunately the steps in CONTRIBUTING.md don't clearly say how to set up the local dev copy of lerna, so I most likely didn't set up the local copy of the repo correctly to ensure each package's dependencies were installed in the correct place.

evocateur

I'm gobsmacked, this is so great!

I will look into the logging issues, but otherwise this looks good to go.

evocateur · 2019-05-13T16:41:39Z

core/otplease/otplease.js

+})
+
+// basic single-entry semaphore
+const semaphore = {


evocateur · 2019-05-13T16:47:41Z

utils/npm-publish/npm-publish.js

@@ -56,7 +57,7 @@ function npmPublish(pkg, tarFilePath, _opts) {
        manifest.publishConfig.tag = opts.tag;
      }

-      return publish(manifest, tarData, opts).catch(err => {
+      return otplease(opts => publish(manifest, tarData, opts), opts, otpCache).catch(err => {
        opts.log.silly("", err);


Perhaps the logging problems are related to the default npmlog export and this.logger from the command instance? I've previously had problems with one or the other not quite responding consistently to "shut up now" and "ok spew now" methods...

evocateur · 2019-05-13T17:39:31Z

As for duplicate npmlog, I only see two (after npm ci, which should be all that's necessary to set up local dev):

$ npm ls -lp npmlog
${PWD}/node_modules/npmlog:npmlog@4.1.2:undefined
${PWD}/node_modules/fsevents/node_modules/npmlog:npmlog@4.1.2:undefined

The http fetch PUT 401 logs are coming from libnpmpublish's opts.log, which should default to npmlog's default export. Maybe...

Could it be pulse-til-done's interval interfering with the prompt? So when @lerna/prompt calls log.pause(), it should probably also call log.clearProgress() (and when it calls log.resume(), call log.showProgress())? The npm source seems to indicate this is necessary.

rbuckton · 2019-05-13T17:54:55Z

@evocateur as I said, the issue was with how I had set up the local repo. I've since fixed my local repo and things work correctly.
This could become a problem in the future if npm is unable to dedupe npmlog due to version clashes with another package. It may be better to pass the logger via opts.log, which is something supported by libnpmpublish via npm-registry-fetch (https://github.com/npm/npm-registry-fetch/blob/latest/README.md#fetch-opts)

evocateur · 2019-05-13T17:55:41Z

Okay, that's also something I was noodling about. I'm in the process of rebasing and fixing the lint errors, etc. Excited to merge!

rbuckton · 2019-05-14T06:23:29Z

Do you have an idea as to when you might publish a new version with this feature?

evocateur · 2019-05-14T13:47:11Z

Was working on that all of yesterday, hit an issue with another PR that I wasted three hours of my life beating my head against the wall investigating. Hopefully today.

…

On May 13, 2019, at 23:23, Ron Buckton ***@***.***> wrote: Do you have an idea as to when you might publish a new version with this feature? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

rbuckton force-pushed the otplease branch from 76fce28 to 4eb1684 Compare May 12, 2019 06:15

rbuckton mentioned this pull request May 12, 2019

Support 2FA for npm via --otp #2076

Closed

9 tasks

rbuckton force-pushed the otplease branch from 4eb1684 to a8b129b Compare May 12, 2019 06:18

rbuckton marked this pull request as ready for review May 12, 2019 20:03

rbuckton force-pushed the otplease branch from a8b129b to cc8cfa8 Compare May 13, 2019 00:27

evocateur approved these changes May 13, 2019

View reviewed changes

Add OTP prompt during publish

9958922

fix lint, complete coverage

37e149a

evocateur force-pushed the otplease branch from 2f8127f to 37e149a Compare May 13, 2019 19:00

evocateur merged commit c56bda1 into lerna:master May 13, 2019

wincent mentioned this pull request Sep 1, 2020

chore: create CONTRIBUTING.md file liferay/liferay-js-toolkit#633

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OTP prompt during publish #2084

Add OTP prompt during publish #2084

rbuckton commented May 12, 2019 •

edited

rbuckton commented May 12, 2019

rbuckton commented May 12, 2019

rbuckton commented May 12, 2019

evocateur left a comment

evocateur May 13, 2019

evocateur May 13, 2019

evocateur commented May 13, 2019

rbuckton commented May 13, 2019

evocateur commented May 13, 2019

rbuckton commented May 14, 2019

evocateur commented May 14, 2019 via email

Add OTP prompt during publish #2084

Add OTP prompt during publish #2084

Conversation

rbuckton commented May 12, 2019 • edited

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

rbuckton commented May 12, 2019

rbuckton commented May 12, 2019

rbuckton commented May 12, 2019

evocateur left a comment

Choose a reason for hiding this comment

evocateur May 13, 2019

Choose a reason for hiding this comment

evocateur May 13, 2019

Choose a reason for hiding this comment

evocateur commented May 13, 2019

rbuckton commented May 13, 2019

evocateur commented May 13, 2019

rbuckton commented May 14, 2019

evocateur commented May 14, 2019 via email

rbuckton commented May 12, 2019 •

edited