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

Release script prompts for NPM 2FA code #12908

Merged
merged 2 commits into from
May 29, 2018

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented May 25, 2018

I haven't really tested this, but in theory, it prompts you for a 2FA code prior to publishing. If you provide a code, it passes it via the --opt flag to npm publish which is how the docs specify you should pass it.

Publishing is quick, and hopefully all of the packages can be published with the same token. But it's possible that it will expire in the middle, which would not be ideal. In that case, you would need to re-run the publish command with modifications to skip the already published packages.

I'm opening this PR for discussion purposes. I'll give it a little more thought before marking it ready for review.

@@ -19,10 +19,17 @@ const push = async ({cwd, dry, packages, version, tag}) => {
throw new Error('The tag `latest` can only be used for stable versions.');
}

// Pass two factor auth code if provided:
// https://docs.npmjs.com/getting-started/using-two-factor-authentication
const twoFactorAuth = opt != null ? `--opt ${opt}` : '';
Copy link
Collaborator

Choose a reason for hiding this comment

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

otp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gah! I read it, multiple times, as "opt" 😦 but you're right.

@bvaughn bvaughn changed the title [WIP] Release script prompts for NPM 2FA code Release script prompts for NPM 2FA code May 29, 2018
@bvaughn
Copy link
Contributor Author

bvaughn commented May 29, 2018

Publishing is quick, and hopefully all of the packages can be published with the same token. But it's possible that it will expire in the middle, which would not be ideal. In that case, you would need to re-run the publish command with modifications to skip the already published packages.

I think this is okay for now. It's already the case if you e.g. don't have access to a package and it fails midway through. So let's follow up if we want to clean up this in general.

@bvaughn bvaughn requested review from gaearon and flarnie May 29, 2018 16:52
@sophiebits
Copy link
Collaborator

I don't expect it to be a large problem in practice. In practice, tokens are good for a minute or two in order to support clock drift.

@bvaughn
Copy link
Contributor Author

bvaughn commented May 29, 2018

Is that an approval? 😄

@sophiebits
Copy link
Collaborator

No, I didn't look at the code.

@bvaughn
Copy link
Contributor Author

bvaughn commented May 29, 2018

Would you...?

@bvaughn bvaughn merged commit 001f9ef into facebook:master May 29, 2018
@bvaughn
Copy link
Contributor Author

bvaughn commented May 29, 2018

Thanks!

@bvaughn bvaughn deleted the release-script-two-factor-auth branch May 29, 2018 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants