-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add OTP prompt during publish #2084
Conversation
@evocateur: The only issue seems to be that the prompt gets lost in among other messages:
As I was typing the OTP the text kept getting replaced with the progress message from |
I thought it might be the newline I added in the prompt, but I'm still seeing the issue after removing it:
It seems like some of the messages sent to |
Ah, I think I figured out the reason. 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm gobsmacked, this is so great!
I will look into the logging issues, but otherwise this looks good to go.
}) | ||
|
||
// basic single-entry semaphore | ||
const semaphore = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant!
@@ -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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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...
As for duplicate $ 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 Could it be |
@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. |
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! |
Do you have an idea as to when you might publish a new version with this feature? |
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.
|
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
Checklist: