feat(publish): Eager prompt for OTP when account-level 2FA is enabled

evocateur · evocateur · commit 4f893d192923 · 2019-07-16T14:28:23.000-07:00
diff --git a/commands/publish/__tests__/get-two-factor-auth-required.test.js b/commands/publish/__tests__/get-two-factor-auth-required.test.js
@@ -0,0 +1,113 @@
+"use strict";
+
+jest.mock("../lib/get-profile-data");
+
+const loggingOutput = require("@lerna-test/logging-output");
+const getProfileData = require("../lib/get-profile-data");
+const getTwoFactorAuthRequired = require("../lib/get-two-factor-auth-required");
+
+getProfileData.mockImplementation(() => Promise.resolve({ tfa: {} }));
+
+expect.extend(require("@lerna-test/figgy-pudding-matchers"));
+
+describe("getTwoFactorAuthRequired", () => {
+  const origConsoleError = console.error;
+
+  beforeEach(() => {
+    console.error = jest.fn();
+  });
+
+  afterEach(() => {
+    console.error = origConsoleError;
+  });
+
+  it("resolves true if tfa.mode === 'auth-and-writes'", async () => {
+    getProfileData.mockResolvedValueOnce({
+      tfa: {
+        mode: "auth-and-writes",
+      },
+    });
+
+    const result = await getTwoFactorAuthRequired();
+    expect(result).toBe(true);
+    expect(getProfileData).toHaveBeenLastCalledWith(expect.figgyPudding({ "fetch-retries": 0 }));
+  });
+
+  it("resolves false if tfa.mode !== 'auth-and-writes'", async () => {
+    getProfileData.mockResolvedValueOnce({
+      tfa: {
+        mode: "auth-only",
+      },
+    });
+
+    const result = await getTwoFactorAuthRequired();
+    expect(result).toBe(false);
+  });
+
+  it("resolves false if tfa.pending === true", async () => {
+    getProfileData.mockResolvedValueOnce({
+      tfa: {
+        pending: true,
+        mode: "ignored",
+      },
+    });
+
+    const result = await getTwoFactorAuthRequired();
+    expect(result).toBe(false);
+  });
+
+  it("resolves false after profile 404", async () => {
+    getProfileData.mockImplementationOnce(() => {
+      const err = new Error("third-party profile fail");
+
+      err.code = "E404";
+
+      return Promise.reject(err);
+    });
+
+    const result = await getTwoFactorAuthRequired();
+
+    expect(result).toBe(false);
+    expect(console.error).not.toHaveBeenCalled();
+  });
+
+  it("resolves false after profile 500", async () => {
+    getProfileData.mockImplementationOnce(() => {
+      const err = new Error("legacy npm Enterprise profile fail");
+
+      err.code = "E500";
+
+      return Promise.reject(err);
+    });
+
+    const opts = new Map([["registry", "such-registry-wow"]]);
+    const result = await getTwoFactorAuthRequired(opts);
+
+    expect(result).toBe(false);
+    expect(loggingOutput("warn")).toContain(
+      `Registry "${opts.get(
+        "registry"
+      )}" does not support 'npm profile get', skipping two-factor auth check...`
+    );
+  });
+
+  it("logs unexpected failure message before throwing validation error", async () => {
+    getProfileData.mockImplementationOnce(() => {
+      const err = new Error("zomg explosions");
+
+      err.code = "E401";
+
+      return Promise.reject(err);
+    });
+
+    try {
+      await getTwoFactorAuthRequired();
+    } catch (err) {
+      expect(err.prefix).toBe("ETWOFACTOR");
+      expect(err.message).toBe("Unable to obtain two-factor auth mode");
+      expect(console.error).toHaveBeenCalledWith("zomg explosions");
+    }
+
+    expect.assertions(3);
+  });
+});
diff --git a/commands/publish/__tests__/publish-canary-no-git-reset.test.js b/commands/publish/__tests__/publish-canary-no-git-reset.test.js
@@ -7,6 +7,7 @@ jest.unmock("@lerna/collect-updates");
 jest.mock("../lib/get-packages-without-license");
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 jest.mock("../lib/git-checkout");
 
 const fs = require("fs-extra");
diff --git a/commands/publish/__tests__/publish-canary.test.js b/commands/publish/__tests__/publish-canary.test.js
@@ -7,6 +7,7 @@ jest.unmock("@lerna/collect-updates");
 jest.mock("../lib/get-packages-without-license");
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 
 const fs = require("fs-extra");
 const path = require("path");
diff --git a/commands/publish/__tests__/publish-command.test.js b/commands/publish/__tests__/publish-command.test.js
@@ -1,9 +1,12 @@
 "use strict";
 
+jest.mock("@lerna/otplease");
+
 // local modules _must_ be explicitly mocked
 jest.mock("../lib/get-packages-without-license");
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 jest.mock("../lib/get-unpublished-packages");
 // FIXME: better mock for version command
 jest.mock("../../version/lib/git-push");
@@ -12,13 +15,15 @@ jest.mock("../../version/lib/is-behind-upstream");
 jest.mock("../../version/lib/remote-branch-exists");
 
 // mocked or stubbed modules
+const otplease = require("@lerna/otplease");
 const npmDistTag = require("@lerna/npm-dist-tag");
 const npmPublish = require("@lerna/npm-publish");
 const packDirectory = require("@lerna/pack-directory");
 const PromptUtilities = require("@lerna/prompt");
 const collectUpdates = require("@lerna/collect-updates");
 const getNpmUsername = require("../lib/get-npm-username");
 const verifyNpmPackageAccess = require("../lib/verify-npm-package-access");
+const getTwoFactorAuthRequired = require("../lib/get-two-factor-auth-required");
 
 // helpers
 const commitChangeToPackage = require("@lerna-test/commit-change-to-package");
@@ -127,6 +132,12 @@ Map {
         "lerna-test",
         expect.figgyPudding({ registry: "https://registry.npmjs.org/" })
       );
+
+      expect(getTwoFactorAuthRequired).toHaveBeenCalled();
+      expect(getTwoFactorAuthRequired).toHaveBeenLastCalledWith(
+        // extra insurance that @lerna/npm-conf is defaulting things correctly
+        expect.figgyPudding({ otp: undefined })
+      );
     });
 
     it("publishes changed independent packages", async () => {
@@ -168,10 +179,15 @@ Map {
   });
 
   describe("--otp", () => {
+    otplease.getOneTimePassword.mockImplementation(() => Promise.resolve("654321"));
+
     it("passes one-time password to npm commands", async () => {
       const testDir = await initFixture("normal");
       const otp = "123456";
 
+      // cli option skips prompt
+      getTwoFactorAuthRequired.mockResolvedValueOnce(true);
+
       await lernaPublish(testDir)("--otp", otp);
 
       expect(npmPublish).toHaveBeenCalledWith(
@@ -180,6 +196,23 @@ Map {
         expect.objectContaining({ otp }),
         expect.objectContaining({ otp })
       );
+      expect(otplease.getOneTimePassword).not.toHaveBeenCalled();
+    });
+
+    it("prompts for OTP when option missing and account-level 2FA enabled", async () => {
+      const testDir = await initFixture("normal");
+
+      getTwoFactorAuthRequired.mockResolvedValueOnce(true);
+
+      await lernaPublish(testDir)();
+
+      expect(npmPublish).toHaveBeenCalledWith(
+        expect.objectContaining({ name: "package-1" }),
+        "/TEMP_DIR/package-1-MOCKED.tgz",
+        expect.objectContaining({ otp: undefined }),
+        expect.objectContaining({ otp: "654321" })
+      );
+      expect(otplease.getOneTimePassword).toHaveBeenLastCalledWith("Enter OTP:");
     });
   });
 
diff --git a/commands/publish/__tests__/publish-from-git.test.js b/commands/publish/__tests__/publish-from-git.test.js
@@ -4,6 +4,7 @@
 jest.mock("../lib/get-packages-without-license");
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 jest.mock("../lib/get-unpublished-packages");
 // FIXME: better mock for version command
 jest.mock("../../version/lib/git-push");
diff --git a/commands/publish/__tests__/publish-from-package.test.js b/commands/publish/__tests__/publish-from-package.test.js
@@ -4,6 +4,7 @@
 jest.mock("../lib/get-packages-without-license");
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 jest.mock("../lib/get-unpublished-packages");
 // FIXME: better mock for version command
 jest.mock("../../version/lib/git-push");
diff --git a/commands/publish/__tests__/publish-licenses.test.js b/commands/publish/__tests__/publish-licenses.test.js
@@ -3,6 +3,7 @@
 // local modules _must_ be explicitly mocked
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 jest.mock("../lib/create-temp-licenses", () => jest.fn(() => Promise.resolve()));
 jest.mock("../lib/remove-temp-licenses", () => jest.fn(() => Promise.resolve()));
 // FIXME: better mock for version command
diff --git a/commands/publish/__tests__/publish-lifecycle-scripts.test.js b/commands/publish/__tests__/publish-lifecycle-scripts.test.js
@@ -4,6 +4,7 @@
 jest.mock("../lib/get-packages-without-license");
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 // FIXME: better mock for version command
 jest.mock("../../version/lib/git-push");
 jest.mock("../../version/lib/is-anything-committed");
diff --git a/commands/publish/__tests__/publish-relative-file-specifiers.test.js b/commands/publish/__tests__/publish-relative-file-specifiers.test.js
@@ -7,6 +7,7 @@ jest.unmock("@lerna/collect-updates");
 jest.mock("../lib/get-packages-without-license");
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 // FIXME: better mock for version command
 jest.mock("../../version/lib/git-push");
 jest.mock("../../version/lib/is-anything-committed");
diff --git a/commands/publish/__tests__/publish-tagging.test.js b/commands/publish/__tests__/publish-tagging.test.js
@@ -4,6 +4,7 @@
 jest.mock("../lib/get-packages-without-license");
 jest.mock("../lib/verify-npm-package-access");
 jest.mock("../lib/get-npm-username");
+jest.mock("../lib/get-two-factor-auth-required");
 // FIXME: better mock for version command
 jest.mock("../../version/lib/git-push");
 jest.mock("../../version/lib/is-anything-committed");
diff --git a/commands/publish/index.js b/commands/publish/index.js
@@ -25,6 +25,7 @@ const runTopologically = require("@lerna/run-topologically");
 const pulseTillDone = require("@lerna/pulse-till-done");
 const versionCommand = require("@lerna/version");
 const prereleaseIdFromVersion = require("@lerna/prerelease-id-from-version");
+const otplease = require("@lerna/otplease");
 
 const createTempLicenses = require("./lib/create-temp-licenses");
 const getCurrentSHA = require("./lib/get-current-sha");
@@ -36,6 +37,7 @@ const getPackagesWithoutLicense = require("./lib/get-packages-without-license");
 const gitCheckout = require("./lib/git-checkout");
 const removeTempLicenses = require("./lib/remove-temp-licenses");
 const verifyNpmPackageAccess = require("./lib/verify-npm-package-access");
+const getTwoFactorAuthRequired = require("./lib/get-two-factor-auth-required");
 
 module.exports = factory;
 
@@ -452,6 +454,13 @@ class PublishCommand extends Command {
       });
     }
 
+    // read profile metadata to determine if account-level 2FA is enabled
+    chain = chain.then(() => getTwoFactorAuthRequired(this.conf.snapshot));
+    chain = chain.then(isRequired => {
+      // notably, this still doesn't handle package-level 2FA requirements
+      this.twoFactorAuthRequired = isRequired;
+    });
+
     return chain;
   }
 
@@ -563,6 +572,19 @@ class PublishCommand extends Command {
       });
   }
 
+  requestOneTimePassword() {
+    // if OTP has already been provided, skip prompt
+    if (this.otpCache.otp) {
+      return;
+    }
+
+    return Promise.resolve()
+      .then(() => otplease.getOneTimePassword("Enter OTP:"))
+      .then(otp => {
+        this.otpCache.otp = otp;
+      });
+  }
+
   topoMapPackages(mapper) {
     // we don't respect --no-sort here, sorry
     return runTopologically(this.packagesToPublish, mapper, {
@@ -643,6 +665,11 @@ class PublishCommand extends Command {
 
     let chain = Promise.resolve();
 
+    // if account-level 2FA is enabled, prime the OTP cache
+    if (this.twoFactorAuthRequired) {
+      chain = chain.then(() => this.requestOneTimePassword());
+    }
+
     const opts = Object.assign(this.conf.snapshot, {
       // distTag defaults to "latest" OR whatever is in pkg.publishConfig.tag
       // if we skip temp tags we should tag with the proper value immediately
diff --git a/commands/publish/lib/__mocks__/get-two-factor-auth-required.js b/commands/publish/lib/__mocks__/get-two-factor-auth-required.js
@@ -0,0 +1,6 @@
+"use strict";
+
+// to mock user modules, you _must_ call `jest.mock('./path/to/module')`
+const mockGetTwoFactorAuthRequired = jest.fn(() => Promise.resolve(false));
+
+module.exports = mockGetTwoFactorAuthRequired;
diff --git a/commands/publish/lib/get-two-factor-auth-required.js b/commands/publish/lib/get-two-factor-auth-required.js
@@ -0,0 +1,50 @@
+"use strict";
+
+const ValidationError = require("@lerna/validation-error");
+const FetchConfig = require("./fetch-config");
+const getProfileData = require("./get-profile-data");
+
+module.exports = getTwoFactorAuthRequired;
+
+function getTwoFactorAuthRequired(_opts) {
+  const opts = FetchConfig(_opts, {
+    // don't wait forever for third-party failures to be dealt with
+    "fetch-retries": 0,
+  });
+
+  opts.log.info("", "Checking two-factor auth mode");
+
+  return getProfileData(opts).then(success, failure);
+
+  function success(result) {
+    opts.log.silly("2FA", result.tfa);
+
+    if (result.tfa.pending) {
+      // if 2FA is pending, it is disabled
+      return false;
+    }
+
+    return result.tfa.mode === "auth-and-writes";
+  }
+
+  function failure(err) {
+    // pass if registry does not support profile endpoint
+    if (err.code === "E500" || err.code === "E404") {
+      // most likely a private registry (npm Enterprise, verdaccio, etc)
+      opts.log.warn(
+        "EREGISTRY",
+        `Registry "${opts.registry}" does not support 'npm profile get', skipping two-factor auth check...`
+      );
+
+      // don't log redundant errors
+      return false;
+    }
+
+    // Log the error cleanly to stderr
+    opts.log.pause();
+    console.error(err.message); // eslint-disable-line no-console
+    opts.log.resume();
+
+    throw new ValidationError("ETWOFACTOR", "Unable to obtain two-factor auth mode");
+  }
+}
diff --git a/commands/publish/package.json b/commands/publish/package.json
@@ -46,6 +46,7 @@
     "@lerna/npm-conf": "file:../../utils/npm-conf",
     "@lerna/npm-dist-tag": "file:../../utils/npm-dist-tag",
     "@lerna/npm-publish": "file:../../utils/npm-publish",
+    "@lerna/otplease": "file:../../core/otplease",
     "@lerna/output": "file:../../utils/output",
     "@lerna/pack-directory": "file:../../utils/pack-directory",
     "@lerna/prerelease-id-from-version": "file:../../utils/prerelease-id-from-version",
diff --git a/package-lock.json b/package-lock.json
