Skip to content

Commit

Permalink
fix(publish): Give up trying to validate third-party registries
Browse files Browse the repository at this point in the history
Fixes #1685
Fixes #1687

And likely a billion others.
  • Loading branch information
evocateur committed Jan 3, 2019
1 parent 25da088 commit b44f2f9
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 24 deletions.
10 changes: 10 additions & 0 deletions commands/publish/__tests__/publish-command.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,16 @@ Set {
expect(logMessages).toContain("Yarn's registry proxy is broken, replacing with public npm registry");
expect(logMessages).toContain("If you don't have an npm token, you should exit and run `npm login`");
});

it("skips validation on any other third-party registry", async () => {
const testDir = await initFixture("normal");
const registry = "https://my-incompatible-registry.com";

await lernaPublish(testDir)("--registry", registry);

const logMessages = loggingOutput("notice");
expect(logMessages).toContain("Skipping all user and access validation due to third-party registry");
});
});

describe("--no-verify-access", () => {
Expand Down
39 changes: 17 additions & 22 deletions commands/publish/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const pMap = require("p-map");
const pPipe = require("p-pipe");
const pReduce = require("p-reduce");
const semver = require("semver");
const getAuth = require("npm-registry-fetch/auth");

const Command = require("@lerna/command");
const describeRef = require("@lerna/describe-ref");
Expand Down Expand Up @@ -66,9 +65,6 @@ class PublishCommand extends Command {
this.logger.info("require-scripts", "enabled");
}

// inverted boolean options
this.verifyAccess = this.options.verifyAccess !== false;

// https://docs.npmjs.com/misc/config#save-prefix
this.savePrefix = this.options.exact ? "" : "^";

Expand Down Expand Up @@ -104,14 +100,6 @@ class PublishCommand extends Command {
this.conf.set("tag", distTag.trim(), "cli");
}

// all consumers need a token
const registry = this.conf.get("registry");
const auth = getAuth(registry, this.conf.snapshot);

if (auth.token) {
this.conf.set("token", auth.token, "cli");
}

this.runPackageLifecycle = createRunner(this.options);

// don't execute recursively if run from a poorly-named script
Expand All @@ -123,13 +111,6 @@ class PublishCommand extends Command {

let chain = Promise.resolve();

// validate user has valid npm credentials first,
// by far the most common form of failed execution
chain = chain.then(() => getNpmUsername(this.conf.snapshot));
chain = chain.then(username => {
// username is necessary for subsequent access check
this.conf.add({ username }, "cmd");
});
chain = chain.then(() => this.findVersionedUpdates());

return chain.then(result => {
Expand Down Expand Up @@ -408,14 +389,28 @@ class PublishCommand extends Command {
prepareRegistryActions() {
let chain = Promise.resolve();

if (this.conf.get("registry") !== "https://registry.npmjs.org/") {
this.logger.notice("", "Skipping all user and access validation due to third-party registry");
this.logger.notice("", "Make sure you're authenticated properly ¯\\_(ツ)_/¯");

return chain;
}

/* istanbul ignore if */
if (process.env.LERNA_INTEGRATION) {
return chain;
}

// if no username was retrieved, don't bother validating
if (this.conf.get("username") && this.verifyAccess) {
chain = chain.then(() => verifyNpmPackageAccess(this.packagesToPublish, this.conf.snapshot));
if (this.options.verifyAccess !== false) {
// validate user has valid npm credentials first,
// by far the most common form of failed execution
chain = chain.then(() => getNpmUsername(this.conf.snapshot));
chain = chain.then(username => {
// if no username was retrieved, don't bother validating
if (username) {
return verifyNpmPackageAccess(this.packagesToPublish, this.conf.snapshot);
}
});
}

return chain;
Expand Down
1 change: 0 additions & 1 deletion commands/publish/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
"figgy-pudding": "^3.5.1",
"fs-extra": "^7.0.0",
"libnpm": "^2.0.1",
"npm-registry-fetch": "^3.8.0",
"p-finally": "^1.0.0",
"p-map": "^1.2.0",
"p-pipe": "^1.2.0",
Expand Down
1 change: 0 additions & 1 deletion package-lock.json

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

3 comments on commit b44f2f9

@ThisIsMissEm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but should it be:

if (this.options.verifyAccess === true) {

Instead?

@evocateur
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because --no-verify-access is the only flag that has any meaning, and the option is otherwise undefined (implicitly true).

The indirection is an unfortunate side-effect of how yargs parses things.

@ThisIsMissEm
Copy link
Contributor

@ThisIsMissEm ThisIsMissEm commented on b44f2f9 Jan 3, 2019 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.