From c56bda18bd0fd8b36d5dbdba6949d7c0c4692086 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Mon, 13 May 2019 12:39:07 -0700 Subject: [PATCH] feat(publish): Add OTP prompt during publish (#2084) Fixes #1091 --- .../publish/__tests__/publish-command.test.js | 6 +- commands/publish/index.js | 5 +- core/otplease/README.md | 7 + core/otplease/__tests__/otplease.test.js | 228 ++++++++++++++++++ core/otplease/otplease.js | 103 ++++++++ core/otplease/package.json | 38 +++ package-lock.json | 8 + .../npm-publish/__tests__/npm-publish.test.js | 3 + utils/npm-publish/npm-publish.js | 5 +- utils/npm-publish/package.json | 1 + 10 files changed, 399 insertions(+), 5 deletions(-) create mode 100644 core/otplease/README.md create mode 100644 core/otplease/__tests__/otplease.test.js create mode 100644 core/otplease/otplease.js create mode 100644 core/otplease/package.json diff --git a/commands/publish/__tests__/publish-command.test.js b/commands/publish/__tests__/publish-command.test.js index 53f5454ecb..eadfade610 100644 --- a/commands/publish/__tests__/publish-command.test.js +++ b/commands/publish/__tests__/publish-command.test.js @@ -176,7 +176,8 @@ Map { expect(npmPublish).toHaveBeenCalledWith( expect.objectContaining({ name: "package-1" }), "/TEMP_DIR/package-1-MOCKED.tgz", - expect.objectContaining({ registry }) + expect.objectContaining({ registry }), + expect.objectContaining({ otp: undefined }) ); }); @@ -189,7 +190,8 @@ Map { expect(npmPublish).toHaveBeenCalledWith( expect.objectContaining({ name: "package-1" }), "/TEMP_DIR/package-1-MOCKED.tgz", - expect.objectContaining({ registry: "https://registry.npmjs.org/" }) + expect.objectContaining({ registry: "https://registry.npmjs.org/" }), + expect.objectContaining({ otp: undefined }) ); const logMessages = loggingOutput("warn"); diff --git a/commands/publish/index.js b/commands/publish/index.js index fbe7343172..abb7e1be2c 100644 --- a/commands/publish/index.js +++ b/commands/publish/index.js @@ -92,6 +92,8 @@ class PublishCommand extends Command { this.logger.verbose("session", npmSession); this.logger.verbose("user-agent", userAgent); + // cache to hold a one-time-password across publishes + this.otpCache = { otp: undefined }; this.conf = npmConf({ lernaCommand: "publish", npmSession, @@ -647,7 +649,8 @@ class PublishCommand extends Command { const preDistTag = this.getPreDistTag(pkg); const tag = !this.options.tempTag && preDistTag ? preDistTag : opts.tag; const pkgOpts = Object.assign({}, opts, { tag }); - return pulseTillDone(npmPublish(pkg, pkg.packed.tarFilePath, pkgOpts)).then(() => { + + return pulseTillDone(npmPublish(pkg, pkg.packed.tarFilePath, pkgOpts, this.otpCache)).then(() => { tracker.success("published", pkg.name, pkg.version); tracker.completeWork(1); diff --git a/core/otplease/README.md b/core/otplease/README.md new file mode 100644 index 0000000000..4a1fa83384 --- /dev/null +++ b/core/otplease/README.md @@ -0,0 +1,7 @@ +# `@lerna/otplease` + +> Prompt for OTP when wrapped Promise fails + +## Usage + +This is an internal lerna library, you probably shouldn't use it directly. \ No newline at end of file diff --git a/core/otplease/__tests__/otplease.test.js b/core/otplease/__tests__/otplease.test.js new file mode 100644 index 0000000000..36a14628bd --- /dev/null +++ b/core/otplease/__tests__/otplease.test.js @@ -0,0 +1,228 @@ +"use strict"; + +jest.mock("@lerna/prompt"); + +// mocked modules +const prompt = require("@lerna/prompt"); + +// file under test +const otplease = require(".."); + +// global mock setup +prompt.input.mockResolvedValue("123456"); + +describe("@lerna/otplease", () => { + const stdinIsTTY = process.stdin.isTTY; + const stdoutIsTTY = process.stdout.isTTY; + + beforeEach(() => { + process.stdin.isTTY = true; + process.stdout.isTTY = true; + }); + + afterEach(() => { + process.stdin.isTTY = stdinIsTTY; + process.stdout.isTTY = stdoutIsTTY; + }); + + it("no error", async () => { + const obj = {}; + const fn = jest.fn(() => obj); + const result = await otplease(fn, {}); + + expect(fn).toHaveBeenCalled(); + expect(prompt.input).not.toHaveBeenCalled(); + expect(result).toBe(obj); + }); + + it("request otp", async () => { + const obj = {}; + const fn = jest.fn(makeTestCallback("123456", obj)); + const result = await otplease(fn, {}); + + expect(fn).toHaveBeenCalledTimes(2); + expect(prompt.input).toHaveBeenCalled(); + expect(result).toBe(obj); + }); + + it("request otp updates cache", async () => { + const otpCache = { otp: undefined }; + const obj = {}; + const fn = jest.fn(makeTestCallback("123456", obj)); + + const result = await otplease(fn, {}, otpCache); + expect(fn).toHaveBeenCalledTimes(2); + expect(prompt.input).toHaveBeenCalled(); + expect(result).toBe(obj); + expect(otpCache.otp).toBe("123456"); + }); + + it("uses cache if opts does not have own otp", async () => { + const otpCache = { otp: "654321" }; + const obj = {}; + const fn = jest.fn(makeTestCallback("654321", obj)); + const result = await otplease(fn, {}, otpCache); + + expect(fn).toHaveBeenCalledTimes(1); + expect(prompt.input).not.toHaveBeenCalled(); + expect(result).toBe(obj); + expect(otpCache.otp).toBe("654321"); + }); + + it("uses explicit otp regardless of cache value", async () => { + const otpCache = { otp: "654321" }; + const obj = {}; + const fn = jest.fn(makeTestCallback("987654", obj)); + const result = await otplease(fn, { otp: "987654" }, otpCache); + + expect(fn).toHaveBeenCalledTimes(1); + expect(prompt.input).not.toHaveBeenCalled(); + expect(result).toBe(obj); + // do not replace cache + expect(otpCache.otp).toBe("654321"); + }); + + it("using cache updated in a different task", async () => { + const otpCache = { otp: undefined }; + const obj = {}; + const fn = jest.fn(makeTestCallback("654321", obj)); + + // enqueue a promise resolution to update the otp at the start of the next turn. + Promise.resolve().then(() => { + otpCache.otp = "654321"; + }); + + // start intial otplease call, 'catch' will happen in next turn *after* the cache is set. + const result = await otplease(fn, {}, otpCache); + expect(fn).toHaveBeenCalledTimes(2); + expect(prompt.input).not.toHaveBeenCalled(); + expect(result).toBe(obj); + }); + + it("semaphore prevents overlapping requests for OTP", async () => { + const otpCache = { otp: undefined }; + + // overlapped calls to otplease that share an otpCache should + // result in the user only being prompted *once* for an OTP. + const obj1 = {}; + const fn1 = jest.fn(makeTestCallback("123456", obj1)); + const p1 = otplease(fn1, {}, otpCache); + + const obj2 = {}; + const fn2 = jest.fn(makeTestCallback("123456", obj2)); + const p2 = otplease(fn2, {}, otpCache); + + const [res1, res2] = await Promise.all([p1, p2]); + + expect(fn1).toHaveBeenCalledTimes(2); + expect(fn2).toHaveBeenCalledTimes(2); + // only prompt once for the two concurrent requests + expect(prompt.input).toHaveBeenCalledTimes(1); + expect(res1).toBe(obj1); + expect(res2).toBe(obj2); + }); + + it("strips whitespace from OTP prompt value", async () => { + prompt.input.mockImplementationOnce((msg, opts) => Promise.resolve(opts.filter(" 121212 "))); + + const obj = {}; + const fn = jest.fn(makeTestCallback("121212", obj)); + const result = await otplease(fn, {}); + + expect(result).toBe(obj); + }); + + it("validates OTP prompt response", async () => { + prompt.input.mockImplementationOnce((msg, opts) => + Promise.resolve(opts.validate("i am the very model of a modern major general")) + ); + + const obj = {}; + const fn = jest.fn(makeTestCallback("343434", obj)); + + try { + await otplease(fn, {}); + } catch (err) { + expect(err.message).toMatch("Must be a valid one-time-password"); + } + + expect.hasAssertions(); + }); + + it("rejects prompt errors", async () => { + prompt.input.mockImplementationOnce(() => Promise.reject(new Error("poopypants"))); + + const obj = {}; + const fn = jest.fn(makeTestCallback("343434", obj)); + + try { + await otplease(fn, {}); + } catch (err) { + expect(err.message).toMatch("poopypants"); + } + + expect.hasAssertions(); + }); + + it("re-throws non-EOTP errors", async () => { + const fn = jest.fn(() => { + const err = new Error("not found"); + err.code = "E404"; + throw err; + }); + + try { + await otplease(fn, {}); + } catch (err) { + expect(err.message).toMatch("not found"); + } + + expect.hasAssertions(); + }); + + it("re-throws E401 errors that do not contain 'one-time pass' in the body", async () => { + const fn = jest.fn(() => { + const err = new Error("auth required"); + err.body = "random arbitrary noise"; + err.code = "E401"; + throw err; + }); + + try { + await otplease(fn, {}); + } catch (err) { + expect(err.message).toMatch("auth required"); + } + + expect.hasAssertions(); + }); + + it.each([["stdin"], ["stdout"]])("re-throws EOTP error when %s is not a TTY", async pipe => { + const fn = jest.fn(() => { + const err = new Error(`non-interactive ${pipe}`); + err.code = "EOTP"; + throw err; + }); + + process[pipe].isTTY = false; + + try { + await otplease(fn); + } catch (err) { + expect(err.message).toBe(`non-interactive ${pipe}`); + } + + expect.hasAssertions(); + }); +}); + +function makeTestCallback(otp, result) { + return opts => { + if (opts.otp !== otp) { + const err = new Error(`oops, received otp ${opts.otp}`); + err.code = "EOTP"; + throw err; + } + return result; + }; +} diff --git a/core/otplease/otplease.js b/core/otplease/otplease.js new file mode 100644 index 0000000000..4109aeb3c8 --- /dev/null +++ b/core/otplease/otplease.js @@ -0,0 +1,103 @@ +"use strict"; + +const figgyPudding = require("figgy-pudding"); +const prompt = require("@lerna/prompt"); + +const OtpPleaseConfig = figgyPudding({ + otp: {}, +}); + +// basic single-entry semaphore +const semaphore = { + wait() { + return new Promise(resolve => { + if (!this._promise) { + // not waiting, block other callers until 'release' is called. + this._promise = new Promise(release => { + this._resolve = release; + }); + resolve(); + } else { + // wait for 'release' to be called and try to lock the semaphore again. + resolve(this._promise.then(() => this.wait())); + } + }); + }, + release() { + const resolve = this._resolve; + // istanbul ignore else + if (resolve) { + this._resolve = undefined; + this._promise = undefined; + // notify waiters that the semaphore has been released. + resolve(); + } + }, +}; + +module.exports = otplease; + +function otplease(fn, _opts, otpCache) { + // NOTE: do not use 'otpCache' as a figgy-pudding provider directly as the + // otp value could change between async wait points. + const opts = OtpPleaseConfig(Object.assign({}, otpCache), _opts); + return attempt(fn, opts, otpCache); +} + +function attempt(fn, opts, otpCache) { + return new Promise(resolve => { + resolve(fn(opts)); + }).catch(err => { + if (err.code !== "EOTP" && !(err.code === "E401" && /one-time pass/.test(err.body))) { + throw err; + } else if (!process.stdin.isTTY || !process.stdout.isTTY) { + throw err; + } else { + // check the cache in case a concurrent caller has already updated the otp. + if (otpCache != null && otpCache.otp != null && otpCache.otp !== opts.otp) { + return attempt(fn, opts.concat(otpCache), otpCache); + } + // only allow one getOneTimePassword attempt at a time to reuse the value + // from the preceeding prompt + return semaphore.wait().then(() => { + // check the cache again in case a previous waiter already updated it. + if (otpCache != null && otpCache.otp != null && otpCache.otp !== opts.otp) { + semaphore.release(); + return attempt(fn, opts.concat({ otp: otpCache.otp }), otpCache); + } + return getOneTimePassword() + .then( + otp => { + // update the otp and release the lock so that waiting + // callers can see the updated otp. + if (otpCache != null) { + // eslint-disable-next-line no-param-reassign + otpCache.otp = otp; + } + semaphore.release(); + return otp; + }, + promptError => { + // release the lock and reject the promise. + semaphore.release(); + return Promise.reject(promptError); + } + ) + .then(otp => { + return fn(opts.concat({ otp })); + }); + }); + } + }); +} + +function getOneTimePassword() { + // Logic taken from npm internals: https://git.io/fNoMe + return prompt.input("This operation requires a one-time password:", { + filter: otp => otp.replace(/\s+/g, ""), + validate: otp => + (otp && /^[\d ]+$|^[A-Fa-f0-9]{64,64}$/.test(otp)) || + "Must be a valid one-time-password. " + + "See https://docs.npmjs.com/getting-started/using-two-factor-authentication", + }); +} diff --git a/core/otplease/package.json b/core/otplease/package.json new file mode 100644 index 0000000000..2c675963ba --- /dev/null +++ b/core/otplease/package.json @@ -0,0 +1,38 @@ +{ + "name": "@lerna/otplease", + "version": "3.13.0", + "description": "Prompt for OTP when wrapped Promise fails", + "keywords": [ + "lerna", + "utils" + ], + "homepage": "https://github.com/lerna/lerna/tree/master/core/otplease#readme", + "license": "MIT", + "author": { + "name": "Daniel Stockman", + "url": "https://github.com/evocateur" + }, + "files": [ + "otplease.js" + ], + "main": "otplease.js", + "engines": { + "node": ">= 6.9.0" + }, + "publishConfig": { + "access": "public" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/lerna/lerna.git", + "directory": "utils/output" + }, + "scripts": { + "test": "echo \"Run tests from root\" && exit 1" + }, + "dependencies": { + "@lerna/prompt": "file:../prompt", + "figgy-pudding": "^3.5.1" + } + } + \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 9f516f7653..c7c888e0a0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1017,6 +1017,7 @@ "@lerna/npm-publish": { "version": "file:utils/npm-publish", "requires": { + "@lerna/otplease": "file:core/otplease", "@lerna/run-lifecycle": "file:utils/run-lifecycle", "figgy-pudding": "^3.5.1", "fs-extra": "^7.0.0", @@ -1035,6 +1036,13 @@ "npmlog": "^4.1.2" } }, + "@lerna/otplease": { + "version": "file:core/otplease", + "requires": { + "@lerna/prompt": "file:core/prompt", + "figgy-pudding": "^3.5.1" + } + }, "@lerna/output": { "version": "file:utils/output", "requires": { diff --git a/utils/npm-publish/__tests__/npm-publish.test.js b/utils/npm-publish/__tests__/npm-publish.test.js index 9b58e2e9b4..df74198e3b 100644 --- a/utils/npm-publish/__tests__/npm-publish.test.js +++ b/utils/npm-publish/__tests__/npm-publish.test.js @@ -1,6 +1,7 @@ "use strict"; jest.mock("@lerna/run-lifecycle"); +jest.mock("@lerna/otplease"); jest.mock("read-package-json"); jest.mock("libnpmpublish"); jest.mock("fs-extra"); @@ -10,6 +11,7 @@ const fs = require("fs-extra"); const { publish } = require("libnpmpublish"); const readJSON = require("read-package-json"); const runLifecycle = require("@lerna/run-lifecycle"); +const otplease = require("@lerna/otplease"); // helpers const path = require("path"); @@ -28,6 +30,7 @@ describe("npm-publish", () => { publish.mockName("libnpmpublish").mockResolvedValue(); readJSON.mockName("read-package-json").mockImplementation((file, cb) => cb(null, mockManifest)); runLifecycle.mockName("@lerna/run-lifecycle").mockResolvedValue(); + otplease.mockName("@lerna/otplease").mockImplementation((cb, opts) => Promise.resolve(cb(opts))); const tarFilePath = "/tmp/test-1.10.100.tgz"; const rootPath = path.normalize("/test"); diff --git a/utils/npm-publish/npm-publish.js b/utils/npm-publish/npm-publish.js index 02daa6ab8f..fea180541f 100644 --- a/utils/npm-publish/npm-publish.js +++ b/utils/npm-publish/npm-publish.js @@ -8,6 +8,7 @@ const readJSON = require("read-package-json"); const figgyPudding = require("figgy-pudding"); const runLifecycle = require("@lerna/run-lifecycle"); const npa = require("npm-package-arg"); +const otplease = require("@lerna/otplease"); module.exports = npmPublish; @@ -30,7 +31,7 @@ const PublishConfig = figgyPudding( } ); -function npmPublish(pkg, tarFilePath, _opts) { +function npmPublish(pkg, tarFilePath, _opts, otpCache) { const { scope } = npa(pkg.name); // pass only the package scope to libnpmpublish const opts = PublishConfig(_opts, { @@ -56,7 +57,7 @@ function npmPublish(pkg, tarFilePath, _opts) { manifest.publishConfig.tag = opts.tag; } - return publish(manifest, tarData, opts).catch(err => { + return otplease(innerOpts => publish(manifest, tarData, innerOpts), opts, otpCache).catch(err => { opts.log.silly("", err); opts.log.error(err.code, (err.body && err.body.error) || err.message); diff --git a/utils/npm-publish/package.json b/utils/npm-publish/package.json index 8354e29b59..082ab0415a 100644 --- a/utils/npm-publish/package.json +++ b/utils/npm-publish/package.json @@ -32,6 +32,7 @@ }, "dependencies": { "@lerna/run-lifecycle": "file:../run-lifecycle", + "@lerna/otplease": "file:../../core/otplease", "figgy-pudding": "^3.5.1", "fs-extra": "^7.0.0", "libnpmpublish": "^1.1.1",