Skip to content

Commit

Permalink
feat(publish): Eager prompt for OTP when account-level 2FA is enabled
Browse files Browse the repository at this point in the history
  • Loading branch information
evocateur committed Jul 16, 2019
1 parent 0501f7a commit 4f893d1
Show file tree
Hide file tree
Showing 15 changed files with 239 additions and 0 deletions.
113 changes: 113 additions & 0 deletions commands/publish/__tests__/get-two-factor-auth-required.test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions commands/publish/__tests__/publish-canary.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
33 changes: 33 additions & 0 deletions commands/publish/__tests__/publish-command.test.js
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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(
Expand All @@ -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:");
});
});

Expand Down
1 change: 1 addition & 0 deletions commands/publish/__tests__/publish-from-git.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions commands/publish/__tests__/publish-from-package.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions commands/publish/__tests__/publish-licenses.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions commands/publish/__tests__/publish-tagging.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
27 changes: 27 additions & 0 deletions commands/publish/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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;

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
50 changes: 50 additions & 0 deletions commands/publish/lib/get-two-factor-auth-required.js
Original file line number Diff line number Diff line change
@@ -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");
}
}
1 change: 1 addition & 0 deletions commands/publish/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 4f893d1

Please sign in to comment.