From 5318f567d04830bdc52185433d7d526ff5879f6f Mon Sep 17 00:00:00 2001
From: Austin Fahsl <austin@fahsl.io>
Date: Wed, 13 Jul 2022 16:38:52 -0400
Subject: [PATCH 1/8] chore: Improve error message when encountering a npm
 automation token

---
 .../__tests__/get-npm-username.test.js        | 19 +++++++++++++++++++
 commands/publish/lib/get-npm-username.js      |  7 +++++++
 2 files changed, 26 insertions(+)

diff --git a/commands/publish/__tests__/get-npm-username.test.js b/commands/publish/__tests__/get-npm-username.test.js
index 858b9ecf39..a1636b1177 100644
--- a/commands/publish/__tests__/get-npm-username.test.js
+++ b/commands/publish/__tests__/get-npm-username.test.js
@@ -68,6 +68,25 @@ describe("getNpmUsername", () => {
     expect(console.error).toHaveBeenCalledWith("third-party whoami fail");
   });
 
+  test("logs failure message when npm automation token is suspected", async () => {
+    fetch.json.mockImplementationOnce(() => {
+      const err = new Error("npm profile fail due to automation token's insufficient permissions");
+
+      err.code = "E403";
+
+      return Promise.reject(err);
+    });
+
+    const opts = { registry: "https://registry.npmjs.org/" };
+
+    await expect(getNpmUsername(opts)).rejects.toThrow(
+      "Cannot verify access when 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)."
+    );
+    expect(console.error).toHaveBeenCalledWith(
+      "npm profile fail due to automation token's insufficient permissions"
+    );
+  });
+
   test("allows third-party registries to fail with a stern warning", async () => {
     fetch.json.mockImplementationOnce(() => {
       const err = new Error("many third-party registries do not support npm whoami");
diff --git a/commands/publish/lib/get-npm-username.js b/commands/publish/lib/get-npm-username.js
index 9ed75949e6..5ab67e9fc5 100644
--- a/commands/publish/lib/get-npm-username.js
+++ b/commands/publish/lib/get-npm-username.js
@@ -54,6 +54,13 @@ function getNpmUsername(options) {
     opts.log.resume();
 
     if (opts.registry === "https://registry.npmjs.org/") {
+      if (err.code === "E403") {
+        throw new ValidationError(
+          "ENEEDAUTH",
+          "Cannot verify access when 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)."
+        );
+      }
+
       throw new ValidationError("EWHOAMI", "Authentication error. Use `npm whoami` to troubleshoot.");
     }
 

From 4b5f2287a28a5c151e4bc4fe7bb4830949935da4 Mon Sep 17 00:00:00 2001
From: Austin Fahsl <austin@fahsl.io>
Date: Wed, 13 Jul 2022 16:49:50 -0400
Subject: [PATCH 2/8] chore: Broaden verbage of error to include generic
 no-access possibility

---
 commands/publish/__tests__/get-npm-username.test.js | 10 ++++------
 commands/publish/lib/get-npm-username.js            |  2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/commands/publish/__tests__/get-npm-username.test.js b/commands/publish/__tests__/get-npm-username.test.js
index a1636b1177..6a115d1414 100644
--- a/commands/publish/__tests__/get-npm-username.test.js
+++ b/commands/publish/__tests__/get-npm-username.test.js
@@ -68,9 +68,9 @@ describe("getNpmUsername", () => {
     expect(console.error).toHaveBeenCalledWith("third-party whoami fail");
   });
 
-  test("logs failure message when npm automation token is suspected", async () => {
+  test("logs failure message when npm returns forbidden response", async () => {
     fetch.json.mockImplementationOnce(() => {
-      const err = new Error("npm profile fail due to automation token's insufficient permissions");
+      const err = new Error("npm profile fail due to insufficient permissions");
 
       err.code = "E403";
 
@@ -80,11 +80,9 @@ describe("getNpmUsername", () => {
     const opts = { registry: "https://registry.npmjs.org/" };
 
     await expect(getNpmUsername(opts)).rejects.toThrow(
-      "Cannot verify access when 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)."
-    );
-    expect(console.error).toHaveBeenCalledWith(
-      "npm profile fail due to automation token's insufficient permissions"
+      "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)."
     );
+    expect(console.error).toHaveBeenCalledWith("npm profile fail due to insufficient permissions");
   });
 
   test("allows third-party registries to fail with a stern warning", async () => {
diff --git a/commands/publish/lib/get-npm-username.js b/commands/publish/lib/get-npm-username.js
index 5ab67e9fc5..32dedfe07b 100644
--- a/commands/publish/lib/get-npm-username.js
+++ b/commands/publish/lib/get-npm-username.js
@@ -57,7 +57,7 @@ function getNpmUsername(options) {
       if (err.code === "E403") {
         throw new ValidationError(
           "ENEEDAUTH",
-          "Cannot verify access when 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. 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)."
         );
       }
 

From dd5960ee72ee0554b0f13882440375c46460e0a6 Mon Sep 17 00:00:00 2001
From: Austin Fahsl <austin@fahsl.io>
Date: Mon, 18 Jul 2022 16:59:04 -0400
Subject: [PATCH 3/8] feat: default verifyAccess to false

---
 .../__tests__/get-npm-username.test.js        |   2 +-
 .../publish/__tests__/publish-command.test.js | 106 ++++++++++++++----
 commands/publish/command.js                   |   9 +-
 commands/publish/index.js                     |  10 +-
 commands/publish/lib/get-npm-username.js      |   2 +-
 5 files changed, 100 insertions(+), 29 deletions(-)

diff --git a/commands/publish/__tests__/get-npm-username.test.js b/commands/publish/__tests__/get-npm-username.test.js
index 6a115d1414..8154f3f985 100644
--- a/commands/publish/__tests__/get-npm-username.test.js
+++ b/commands/publish/__tests__/get-npm-username.test.js
@@ -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");
   });
diff --git a/commands/publish/__tests__/publish-command.test.js b/commands/publish/__tests__/publish-command.test.js
index ef6dd31641..0fada621ae 100644
--- a/commands/publish/__tests__/publish-command.test.js
+++ b/commands/publish/__tests__/publish-command.test.js
@@ -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
@@ -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" }),
@@ -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", () => {
@@ -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");
 
diff --git a/commands/publish/command.js b/commands/publish/command.js
index ec2d3e9b0d..2d2273dadc 100644
--- a/commands/publish/command.js
+++ b/commands/publish/command.js
@@ -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);
diff --git a/commands/publish/index.js b/commands/publish/index.js
index 16d12a2c88..9b19f7f86a 100644
--- a/commands/publish/index.js
+++ b/commands/publish/index.js
@@ -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() {
@@ -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");
diff --git a/commands/publish/lib/get-npm-username.js b/commands/publish/lib/get-npm-username.js
index 32dedfe07b..68e81a95f0 100644
--- a/commands/publish/lib/get-npm-username.js
+++ b/commands/publish/lib/get-npm-username.js
@@ -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)."
         );
       }
 

From cf79b9d7e0a5744c461c9281ba3bbd94e7bfe11a Mon Sep 17 00:00:00 2001
From: Austin Fahsl <austin@fahsl.io>
Date: Mon, 18 Jul 2022 18:21:51 -0400
Subject: [PATCH 4/8] chore(docs): update publish readme to describe new
 behavior

---
 commands/publish/README.md | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/commands/publish/README.md b/commands/publish/README.md
index d9c67fda4c..7fdc322363 100644
--- a/commands/publish/README.md
+++ b/commands/publish/README.md
@@ -26,8 +26,6 @@ During all publish operations, appropriate [lifecycle scripts](#lifecycle-script
 
 Check out [Per-Package Configuration](#per-package-configuration) for more details about publishing scoped packages, custom registries, and custom dist-tags.
 
-> If you're using [npm automation access token](https://docs.npmjs.com/creating-and-viewing-access-tokens#creating-access-tokens) please remember to [disable lerna access verification feature](#--no-verify-access). Automation token doesn't grant permissions needed for the verification to be successful. [Click here to read more about this issue](https://github.com/lerna/lerna/issues/2788).
-
 ## Positionals
 
 ### bump `from-git`
@@ -59,7 +57,7 @@ This is useful when a previous `lerna publish` failed to publish all packages to
 - [`--legacy-auth`](#--legacy-auth)
 - [`--no-git-reset`](#--no-git-reset)
 - [`--no-granular-pathspec`](#--no-granular-pathspec)
-- [`--no-verify-access`](#--no-verify-access)
+- [`--verify-access`](#--verify-access)
 - [`--otp`](#--otp)
 - [`--preid`](#--preid)
 - [`--pre-dist-tag <tag>`](#--pre-dist-tag-tag)
@@ -199,15 +197,14 @@ This option makes the most sense configured in lerna.json, as you really don't w
 
 The root-level configuration is intentional, as this also covers the [identically-named option in `lerna version`](https://github.com/lerna/lerna/tree/main/commands/version#--no-granular-pathspec).
 
-### `--no-verify-access`
-
-By default, `lerna` will verify the logged-in npm user's access to the packages about to be published. Passing this flag will disable that check.
+### `--verify-access`
 
-If you are using a third-party registry that does not support `npm access ls-packages`, you will need to pass this flag (or set `command.publish.verifyAccess` to `false` in lerna.json).
+By default, `lerna` will not verify the logged-in npm user's access to the packages about to be published. Passing this flag will cause `lerna` to proactively perform this verification before it attempts to publish any packages.
 
-> Please use with caution
+You should NOT use this option if:
 
-> For the time being, use this flag/option always when you're handling NPM authorization with the use of [automation access token](https://docs.npmjs.com/creating-and-viewing-access-tokens#creating-access-tokens). [Click here to read more about this issue](https://github.com/lerna/lerna/issues/2788).
+1.  You are using a third-party registry that does not support `npm access ls-packages`
+2.  You are using an authentication token without read access, such as a [npm automation access token](https://docs.npmjs.com/creating-and-viewing-access-tokens#creating-access-tokens)
 
 ### `--otp`
 
@@ -297,6 +294,10 @@ Useful in [Continuous integration (CI)](https://en.wikipedia.org/wiki/Continuous
 
 ## Deprecated Options
 
+### `--no-verify-access`
+
+Access verification is now off by default, so `--no-verify-access` is not needed. To opt-in to access verification, use [`--verify-access`](#--verify-access).
+
 ### `--skip-npm`
 
 Call [`lerna version`](https://github.com/lerna/lerna/tree/main/commands/version#readme) directly, instead.

From 7747655b729aefe9a9ea8ea374e3163cb8e7748e Mon Sep 17 00:00:00 2001
From: James Henry <james@henry.sc>
Date: Fri, 22 Jul 2022 18:13:53 +0400
Subject: [PATCH 5/8] chore: update commands/publish/index.js

---
 commands/publish/index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/publish/index.js b/commands/publish/index.js
index 9b19f7f86a..f123f352ef 100644
--- a/commands/publish/index.js
+++ b/commands/publish/index.js
@@ -98,7 +98,7 @@ class PublishCommand extends Command {
     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."
+        "--verify-access=false and --no-verify-access are no longer needed, because the legacy preemptive access verification is now disabled by default. Requests will fail with appropriate errors when not authorized correctly."
       );
     }
 

From 444036c471f0ae5d78d60509ecd02e8b862175b7 Mon Sep 17 00:00:00 2001
From: James Henry <james@henry.sc>
Date: Fri, 22 Jul 2022 18:16:24 +0400
Subject: [PATCH 6/8] chore: update
 commands/publish/__tests__/publish-command.test.js

---
 commands/publish/__tests__/publish-command.test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/publish/__tests__/publish-command.test.js b/commands/publish/__tests__/publish-command.test.js
index 0fada621ae..5d4434870c 100644
--- a/commands/publish/__tests__/publish-command.test.js
+++ b/commands/publish/__tests__/publish-command.test.js
@@ -371,7 +371,7 @@ Map {
 
       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."
+        "--verify-access=false and --no-verify-access are no longer needed, because the legacy preemptive access verification is now disabled by default. Requests will fail with appropriate errors when not authorized correctly."
       );
     });
 

From c3873115a0022afdf1b0cf94ab6ea31af0799155 Mon Sep 17 00:00:00 2001
From: James Henry <james@henry.sc>
Date: Fri, 22 Jul 2022 18:18:45 +0400
Subject: [PATCH 7/8] chore: update commands/publish/README.md

---
 commands/publish/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/publish/README.md b/commands/publish/README.md
index 7fdc322363..2bae8e33e5 100644
--- a/commands/publish/README.md
+++ b/commands/publish/README.md
@@ -296,7 +296,7 @@ Useful in [Continuous integration (CI)](https://en.wikipedia.org/wiki/Continuous
 
 ### `--no-verify-access`
 
-Access verification is now off by default, so `--no-verify-access` is not needed. To opt-in to access verification, use [`--verify-access`](#--verify-access).
+The legacy preemptive access verification is now off by default, so `--no-verify-access` is not needed. Requests will fail with appropriate errors when not authorized correctly. To opt-in to the legacy access verification, use [`--verify-access`](#--verify-access).
 
 ### `--skip-npm`
 

From ba36fd755e7a1308af6a96cbd06715af2c86f6d0 Mon Sep 17 00:00:00 2001
From: James Henry <james@henry.sc>
Date: Fri, 22 Jul 2022 18:23:11 +0400
Subject: [PATCH 8/8] chore: update commands/publish/README.md

---
 commands/publish/README.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/commands/publish/README.md b/commands/publish/README.md
index 2bae8e33e5..7337294241 100644
--- a/commands/publish/README.md
+++ b/commands/publish/README.md
@@ -199,7 +199,9 @@ The root-level configuration is intentional, as this also covers the [identicall
 
 ### `--verify-access`
 
-By default, `lerna` will not verify the logged-in npm user's access to the packages about to be published. Passing this flag will cause `lerna` to proactively perform this verification before it attempts to publish any packages.
+Historically, `lerna` attempted to fast-fail on authorization/authentication issues by performing some preemptive npm API requests using the given token. These days, however, there are multiple types of tokens that npm supports and they have varying levels of access rights, so there is no one-size fits all solution for this preemptive check and it is more appropriate to allow requests to npm to simply fail with appropriate errors for the given token. For this reason, the legacy `--verify-access` behavior is disabled by default and will likely be removed in a future major version.
+
+For now, though, if you pass this flag you can opt into the legacy behavior and `lerna` will preemptively perform this verification before it attempts to publish any packages.
 
 You should NOT use this option if: