Skip to content

Commit

Permalink
feat: default verifyAccess to false
Browse files Browse the repository at this point in the history
  • Loading branch information
fahslaj committed Jul 19, 2022
1 parent 29c8119 commit 1bd22b7
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 29 deletions.
2 changes: 1 addition & 1 deletion commands/publish/__tests__/get-npm-username.test.js
Expand Up @@ -80,7 +80,7 @@ describe("getNpmUsername", () => {
const opts = { registry: "https://registry.npmjs.org/" };

await expect(getNpmUsername(opts)).rejects.toThrow(
"Access verification failed. If you are authenticating with a npm automation token, set `command.publish.verifyAccess=false` in your lerna.json to skip this verification, or use a different type of npm access token (https://docs.npmjs.com/creating-and-viewing-access-tokens)."
"Access verification failed. Ensure that your npm access token has both read and write access, or remove the verifyAccess option to skip this verification. Note that npm automation tokens do NOT have read access (https://docs.npmjs.com/creating-and-viewing-access-tokens)."
);
expect(console.error).toHaveBeenCalledWith("npm profile fail due to insufficient permissions");
});
Expand Down
106 changes: 87 additions & 19 deletions commands/publish/__tests__/publish-command.test.js
Expand Up @@ -124,23 +124,8 @@ Map {
expect(npmDistTag.remove).not.toHaveBeenCalled();
expect(npmDistTag.add).not.toHaveBeenCalled();

expect(getNpmUsername).toHaveBeenCalled();
expect(getNpmUsername).toHaveBeenLastCalledWith(
expect.objectContaining({ registry: "https://registry.npmjs.org/" })
);

expect(verifyNpmPackageAccess).toHaveBeenCalled();
expect(verifyNpmPackageAccess).toHaveBeenLastCalledWith(
expect.any(Array),
"lerna-test",
expect.objectContaining({ registry: "https://registry.npmjs.org/" })
);

expect(getTwoFactorAuthRequired).toHaveBeenCalled();
expect(getTwoFactorAuthRequired).toHaveBeenLastCalledWith(
// extra insurance that @lerna/npm-conf is defaulting things correctly
expect.objectContaining({ otp: undefined })
);
expect(getNpmUsername).not.toHaveBeenCalled();
expect(verifyNpmPackageAccess).not.toHaveBeenCalled();

expect(gitCheckout).toHaveBeenCalledWith(
// the list of changed files has been asserted many times already
Expand Down Expand Up @@ -228,12 +213,12 @@ Map {
expect(getOneTimePassword).not.toHaveBeenCalled();
});

it("prompts for OTP when option missing and account-level 2FA enabled", async () => {
it("prompts for OTP when option missing, account-level 2FA enabled, and verify access is true", async () => {
const testDir = await initFixture("normal");

getTwoFactorAuthRequired.mockResolvedValueOnce(true);

await lernaPublish(testDir)();
await lernaPublish(testDir)("--verify-access", true);

expect(npmPublish).toHaveBeenCalledWith(
expect.objectContaining({ name: "package-1" }),
Expand All @@ -243,6 +228,22 @@ Map {
);
expect(getOneTimePassword).toHaveBeenLastCalledWith("Enter OTP:");
});

it("defers OTP prompt when option missing, account-level 2FA enabled, and verify access is not true", 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: undefined })
);
expect(getOneTimePassword).not.toHaveBeenCalled();
});
});

describe("--legacy-auth", () => {
Expand Down Expand Up @@ -306,7 +307,74 @@ Map {
});
});

describe("--verify-access", () => {
it("publishes packages after verifying the user's access to each package", async () => {
const testDir = await initFixture("normal");

await lernaPublish(testDir)("--verify-access");

expect(promptConfirmation).toHaveBeenLastCalledWith("Are you sure you want to publish these packages?");
expect(packDirectory.registry).toMatchInlineSnapshot(`
Set {
"package-1",
"package-3",
"package-4",
"package-2",
}
`);
expect(npmPublish.registry).toMatchInlineSnapshot(`
Map {
"package-1" => "latest",
"package-3" => "latest",
"package-4" => "latest",
"package-2" => "latest",
}
`);
expect(npmPublish.order()).toEqual([
"package-1",
"package-3",
"package-4",
"package-2",
// package-5 is private
]);
expect(npmDistTag.remove).not.toHaveBeenCalled();
expect(npmDistTag.add).not.toHaveBeenCalled();

expect(getNpmUsername).toHaveBeenCalled();
expect(getNpmUsername).toHaveBeenLastCalledWith(
expect.objectContaining({ registry: "https://registry.npmjs.org/" })
);

expect(verifyNpmPackageAccess).toHaveBeenCalled();
expect(verifyNpmPackageAccess).toHaveBeenLastCalledWith(
expect.any(Array),
"lerna-test",
expect.objectContaining({ registry: "https://registry.npmjs.org/" })
);

expect(getTwoFactorAuthRequired).toHaveBeenCalled();
expect(getTwoFactorAuthRequired).toHaveBeenLastCalledWith(expect.objectContaining({ otp: undefined }));

expect(gitCheckout).toHaveBeenCalledWith(
expect.any(Array),
{ granularPathspec: true },
{ cwd: testDir }
);
});
});

describe("--no-verify-access", () => {
it("shows warning that this is the default behavior and that this option is no longer needed", async () => {
const cwd = await initFixture("normal");

await lernaPublish(cwd)("--no-verify-access");

const logMessages = loggingOutput("warn");
expect(logMessages).toContain(
"--verify-access=false and --no-verify-access are no longer needed, since skipping access verification is now the default behavior."
);
});

it("skips package access verification", async () => {
const cwd = await initFixture("normal");

Expand Down
9 changes: 2 additions & 7 deletions commands/publish/command.js
Expand Up @@ -102,19 +102,14 @@ exports.builder = (yargs) => {
type: "boolean",
},
"no-verify-access": {
// proxy for --verify-access
describe: "Do not verify package read-write access for current npm user.",
type: "boolean",
},
"verify-access": {
// proxy for --no-verify-access
hidden: true,
describe: "Verify package read-write access for current npm user.",
type: "boolean",
},
// y: {
// describe: "Skip all confirmation prompts.",
// alias: "yes",
// type: "boolean",
// },
};

composeVersionOptions(yargs);
Expand Down
10 changes: 9 additions & 1 deletion commands/publish/index.js
Expand Up @@ -82,10 +82,11 @@ class PublishCommand extends Command {

// inverted boolean options are only respected if prefixed with `--no-`, e.g. `--no-verify-access`
this.gitReset = gitReset !== false;
this.verifyAccess = verifyAccess !== false;

// consumed by npm-registry-fetch (via libnpmpublish)
this.npmSession = crypto.randomBytes(8).toString("hex");

this.verifyAccess = verifyAccess;
}

get userAgent() {
Expand All @@ -94,6 +95,13 @@ class PublishCommand extends Command {
}

initialize() {
if (this.options.verifyAccess === false) {
this.logger.warn(
"verify-access",
"--verify-access=false and --no-verify-access are no longer needed, since skipping access verification is now the default behavior."
);
}

if (this.options.skipNpm) {
// TODO: remove in next major release
this.logger.warn("deprecated", "Instead of --skip-npm, call `lerna version` directly");
Expand Down
2 changes: 1 addition & 1 deletion commands/publish/lib/get-npm-username.js
Expand Up @@ -57,7 +57,7 @@ function getNpmUsername(options) {
if (err.code === "E403") {
throw new ValidationError(
"ENEEDAUTH",
"Access verification failed. If you are authenticating with a npm automation token, set `command.publish.verifyAccess=false` in your lerna.json to skip this verification, or use a different type of npm access token (https://docs.npmjs.com/creating-and-viewing-access-tokens)."
"Access verification failed. Ensure that your npm access token has both read and write access, or remove the verifyAccess option to skip this verification. Note that npm automation tokens do NOT have read access (https://docs.npmjs.com/creating-and-viewing-access-tokens)."
);
}

Expand Down

0 comments on commit 1bd22b7

Please sign in to comment.