Skip to content

Commit

Permalink
fix(publish): Short-circuit retries for npm username validation
Browse files Browse the repository at this point in the history
  • Loading branch information
evocateur committed Dec 19, 2018
1 parent 405b094 commit ca4dd95
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 32 deletions.
11 changes: 7 additions & 4 deletions commands/publish/__tests__/get-npm-username.test.js
@@ -1,13 +1,15 @@
"use strict";

jest.mock("npm-registry-fetch");
jest.mock("libnpm/fetch");

const fetch = require("npm-registry-fetch");
const fetch = require("libnpm/fetch");
const loggingOutput = require("@lerna-test/logging-output");
const getNpmUsername = require("../lib/get-npm-username");

fetch.json.mockImplementation(() => Promise.resolve({ username: "lerna-test" }));

expect.extend(require("@lerna-test/figgy-pudding-matchers"));

describe("getNpmUsername", () => {
const origConsoleError = console.error;

Expand All @@ -27,12 +29,13 @@ describe("getNpmUsername", () => {

return Promise.reject(err);
});
const opts = { such: "npm-conf", wow: true };
const opts = new Map();
opts.set("registry", "such-config-wow");

const username = await getNpmUsername(opts);

expect(username).toBe("lerna-test");
expect(fetch.json).toHaveBeenLastCalledWith("/-/whoami", opts);
expect(fetch.json).toHaveBeenLastCalledWith("/-/whoami", expect.figgyPudding({ "fetch-retries": 0 }));
});

test("throws an error when successful fetch yields empty username", async () => {
Expand Down
4 changes: 3 additions & 1 deletion commands/publish/__tests__/publish-command.test.js
Expand Up @@ -115,7 +115,9 @@ Set {
expect(npmDistTag.add).not.toHaveBeenCalled();

expect(getNpmUsername).toHaveBeenCalled();
expect(getNpmUsername.registry.get(testDir).get("registry")).toBe("https://registry.npmjs.org/");
expect(getNpmUsername).toHaveBeenLastCalledWith(
expect.figgyPudding({ registry: "https://registry.npmjs.org/" })
);

expect(verifyNpmPackageAccess).toHaveBeenCalled();
expect(verifyNpmPackageAccess).toHaveBeenLastCalledWith(
Expand Down
2 changes: 1 addition & 1 deletion commands/publish/index.js
Expand Up @@ -116,7 +116,7 @@ class PublishCommand extends Command {

// validate user has valid npm credentials first,
// by far the most common form of failed execution
chain = chain.then(() => getNpmUsername(this.conf));
chain = chain.then(() => getNpmUsername(this.conf.snapshot));
chain = chain.then(username => {
// username is necessary for subsequent access check
this.conf.add({ username }, "cmd");
Expand Down
14 changes: 1 addition & 13 deletions commands/publish/lib/__mocks__/get-npm-username.js
@@ -1,18 +1,6 @@
"use strict";

const registry = new Map();

// to mock user modules, you _must_ call `jest.mock('./path/to/module')`
const mockGetNpmUsername = jest.fn(opts => {
registry.set(opts.prefix, opts);

return Promise.resolve("lerna-test");
});

// keep test data isolated
afterEach(() => {
registry.clear();
});
const mockGetNpmUsername = jest.fn(() => Promise.resolve("lerna-test"));

module.exports = mockGetNpmUsername;
module.exports.registry = registry;
31 changes: 18 additions & 13 deletions commands/publish/lib/get-npm-username.js
@@ -1,18 +1,23 @@
"use strict";

const log = require("libnpm/log");
const fetch = require("npm-registry-fetch");
const fetch = require("libnpm/fetch");
const ValidationError = require("@lerna/validation-error");
const FetchConfig = require("./fetch-config");

module.exports = getNpmUsername;

function getNpmUsername(opts) {
log.info("", "Verifying npm credentials");
function getNpmUsername(_opts) {
const opts = FetchConfig(_opts, {
// don't wait forever for third-party failures to be dealt with
"fetch-retries": 0,
});

opts.log.info("", "Verifying npm credentials");

return getProfileData(opts).then(success, failure);

function success(result) {
log.silly("npm whoami", "received %j", result);
opts.log.silly("npm whoami", "received %j", result);

if (!result.username) {
throw new ValidationError(
Expand All @@ -27,28 +32,28 @@ function getNpmUsername(opts) {
// catch request errors, not auth expired errors
function failure(err) {
// Log the error cleanly to stderr
log.pause();
opts.log.pause();
console.error(err.message); // eslint-disable-line no-console
log.resume();
opts.log.resume();

if (opts.get("registry") === "https://registry.npmjs.org/") {
if (opts.registry === "https://registry.npmjs.org/") {
throw new ValidationError("EWHOAMI", "Authentication error. Use `npm whoami` to troubleshoot.");
}

log.warn(
opts.log.warn(
"EWHOAMI",
"Unable to determine npm username from third-party registry, this command will likely fail soon!"
);
}
}

function getProfileData(opts) {
log.verbose("", "Retrieving npm user profile");
opts.log.verbose("", "Retrieving npm user profile");

return fetch
.json("/-/npm/v1/user", opts)
.then(data => {
log.silly("npm profile get", "received %j", data);
opts.log.silly("npm profile get", "received %j", data);

const result = {
// remap to match legacy whoami format
Expand All @@ -70,10 +75,10 @@ function getProfileData(opts) {
}

function getWhoAmI(opts) {
log.verbose("", "Retrieving npm username");
opts.log.verbose("", "Retrieving npm username");

return fetch.json("/-/whoami", opts).then(data => {
log.silly("npm whoami", "received %j", data);
opts.log.silly("npm whoami", "received %j", data);

// { username: String }
return data;
Expand Down

0 comments on commit ca4dd95

Please sign in to comment.