Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): Support two factor authentication for NPM accounts #6555

Merged
merged 7 commits into from Oct 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -32,6 +32,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa

[#5322](https://github.com/yarnpkg/yarn/pull/5322) - [**Karolis Narkevicius**](https://twitter.com/KidkArolis)

- Adds 2FA (Two Factor Authentication) support to publish & alike

[#6555](https://github.com/yarnpkg/yarn/pull/6555) - [**Krzysztof Zbudniewek**](https://github.com/neonowy)

- Fixes how the `files` property is interpreted to bring it in line with npm

[#6562](https://github.com/yarnpkg/yarn/pull/6562) - [**Bertrand Marron**](https://github.com/tusbar)
Expand Down
15 changes: 15 additions & 0 deletions __tests__/registries/npm-registry.js
Expand Up @@ -85,6 +85,10 @@ describe('request', () => {
const npmRegistry = new NpmRegistry(testCwd, mockRegistries, mockRequestManager, mockReporter, true, []);
npmRegistry.config = config;
return {
setOtp(otp: string) {
npmRegistry.setOtp(otp);
},

request(url: string, options: Object, packageName: string): Object {
npmRegistry.request(url, options, packageName);
const lastIndex = mockRequestManager.request.mock.calls.length - 1;
Expand All @@ -101,6 +105,17 @@ describe('request', () => {
expect(requestParams.url).toBe(url);
});

test('should add `npm-otp` header', () => {
const url = 'https://registry.npmjs.org/yarn';
const config = {};
const registry = createRegistry(config);

registry.setOtp('123 456');

const requestParams = registry.request(url);
expect(requestParams.headers['npm-otp']).toBe('123 456');
});

const testCases = [
{
title: 'using npm as default registry and using private registry for scoped packages',
Expand Down
43 changes: 43 additions & 0 deletions __tests__/util/request-manager.js
@@ -1,6 +1,7 @@
/* @flow */
/* eslint max-len: 0 */

import {OneTimePasswordError} from '../../src/errors.js';
import {Reporter} from '../../src/reporters/index.js';
import Config from '../../src/config.js';
import * as fs from '../../src/util/fs.js';
Expand Down Expand Up @@ -209,6 +210,48 @@ for (const statusCode of [403, 442]) {
});
}

test('RequestManager.execute one time password error on npm request', async () => {
jest.resetModules();
jest.mock('request', factory => options => {
options.callback(
'',
{statusCode: 401, headers: {'www-authenticate': 'otp'}},
{error: 'You must provide a one-time pass. Upgrade your client to npm@latest in order to use 2FA.'},
);
return {
on: () => {},
};
});

try {
const config = await Config.create({});
await config.requestManager.request({
url: 'https://registry.npmjs.org/yarn',
});
} catch (err) {
expect(err).toBeInstanceOf(OneTimePasswordError);
}
});

test('RequestManager.execute one time password error on npm login request', async () => {
jest.resetModules();
jest.mock('request', factory => options => {
options.callback('', {statusCode: 401, headers: {'www-authenticate': 'otp'}}, {ok: false});
return {
on: () => {},
};
});

try {
const config = await Config.create({});
await config.requestManager.request({
url: 'https://registry.npmjs.org/-/user/org.couchdb.user:user',
});
} catch (err) {
expect(err).toBeInstanceOf(OneTimePasswordError);
}
});

// Cloudflare will occasionally return an html response with a 500 status code on some calls
for (const statusCode of [408, 500, 542]) {
test(`RequestManager.execute retries on ${statusCode} error`, async () => {
Expand Down
8 changes: 8 additions & 0 deletions src/cli/commands/login.js
Expand Up @@ -36,6 +36,10 @@ async function getCredentials(
return {username, email};
}

export function getOneTimePassword(reporter: Reporter): Promise<string> {
return reporter.question(reporter.lang('npmOneTimePassword'));
}

export async function getToken(
config: Config,
reporter: Reporter,
Expand All @@ -45,6 +49,10 @@ export async function getToken(
): Promise<() => Promise<void>> {
const auth = registry ? config.registries.npm.getAuthByRegistry(registry) : config.registries.npm.getAuth(name);

if (config.otp) {
config.registries.npm.setOtp(config.otp);
}

if (auth) {
config.registries.npm.setToken(auth);
return function revoke(): Promise<void> {
Expand Down
2 changes: 2 additions & 0 deletions src/cli/index.js
Expand Up @@ -122,6 +122,7 @@ export async function main({
);
commander.option('--no-node-version-check', 'do not warn when using a potentially unsupported Node version');
commander.option('--focus', 'Focus on a single workspace by installing remote copies of its sibling workspaces.');
commander.option('--otp <otpcode>', 'one-time password for two factor authentication');

// if -v is the first command, then always exit after returning the version
if (args[0] === '-v') {
Expand Down Expand Up @@ -527,6 +528,7 @@ export async function main({
nonInteractive: commander.nonInteractive,
updateChecksums: commander.updateChecksums,
focus: commander.focus,
otp: commander.otp,
})
.then(() => {
// lockfile check must happen after config.init sets lockfileFolder
Expand Down
6 changes: 6 additions & 0 deletions src/config.js
Expand Up @@ -68,6 +68,8 @@ export type ConfigOptions = {
updateChecksums?: boolean,

focus?: boolean,

otp?: string,
};

type PackageMetadata = {
Expand Down Expand Up @@ -207,6 +209,8 @@ export default class Config {

autoAddIntegrity: boolean;

otp: ?string;

/**
* Execute a promise produced by factory if it doesn't exist in our cache with
* the associated key.
Expand Down Expand Up @@ -495,6 +499,8 @@ export default class Config {

this.focus = !!opts.focus;
this.focusedWorkspaceName = '';

this.otp = opts.otp || '';
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/errors.js
Expand Up @@ -33,3 +33,5 @@ export class ResponseError extends Error {

responseCode: number;
}

export class OneTimePasswordError extends Error {}
7 changes: 7 additions & 0 deletions src/registries/base-registry.js
Expand Up @@ -65,6 +65,9 @@ export default class BaseRegistry {
//
token: string;

//
otp: string;

//
cwd: string;

Expand All @@ -81,6 +84,10 @@ export default class BaseRegistry {
this.token = token;
}

setOtp(otp: string) {
this.otp = otp;
}

getOption(key: string): mixed {
return this.config[key];
}
Expand Down
47 changes: 35 additions & 12 deletions src/registries/npm-registry.js
Expand Up @@ -15,6 +15,8 @@ import {addSuffix} from '../util/misc';
import {getPosixPath, resolveWithHome} from '../util/path';
import normalizeUrl from 'normalize-url';
import {default as userHome, home} from '../util/user-home-dir';
import {MessageError, OneTimePasswordError} from '../errors.js';
import {getOneTimePassword} from '../cli/commands/login.js';
import path from 'path';
import url from 'url';
import ini from 'ini';
Expand Down Expand Up @@ -133,7 +135,7 @@ export default class NpmRegistry extends Registry {
return (requestToRegistryHost || requestToYarn) && (requestToRegistryPath || customHostSuffixInUse);
}

request(pathname: string, opts?: RegistryRequestOptions = {}, packageName: ?string): Promise<*> {
async request(pathname: string, opts?: RegistryRequestOptions = {}, packageName: ?string): Promise<*> {
// packageName needs to be escaped when if it is passed
const packageIdent = (packageName && NpmRegistry.escapeName(packageName)) || pathname;
const registry = opts.registry || this.getRegistry(packageIdent);
Expand Down Expand Up @@ -161,17 +163,38 @@ export default class NpmRegistry extends Registry {
}
}

return this.requestManager.request({
url: requestUrl,
method: opts.method,
body: opts.body,
auth: opts.auth,
headers,
json: !opts.buffer,
buffer: opts.buffer,
process: opts.process,
gzip: true,
});
if (this.otp) {
headers['npm-otp'] = this.otp;
}

try {
return await this.requestManager.request({
url: requestUrl,
method: opts.method,
body: opts.body,
auth: opts.auth,
headers,
json: !opts.buffer,
buffer: opts.buffer,
process: opts.process,
gzip: true,
});
} catch (error) {
if (error instanceof OneTimePasswordError) {
if (this.otp) {
throw new MessageError(this.reporter.lang('incorrectOneTimePassword'));
}

this.reporter.info(this.reporter.lang('twoFactorAuthenticationEnabled'));
this.otp = await getOneTimePassword(this.reporter);

this.requestManager.clearCache();

return this.request(pathname, opts, packageName);
} else {
throw error;
}
}
}

requestNeedsAuth(requestUrl: string): boolean {
Expand Down
3 changes: 3 additions & 0 deletions src/reporters/lang/en.js
Expand Up @@ -311,6 +311,7 @@ const messages = {
npmUsername: 'npm username',
npmPassword: 'npm password',
npmEmail: 'npm email',
npmOneTimePassword: 'npm one-time password',

loggingIn: 'Logging in',
loggedIn: 'Logged in.',
Expand All @@ -322,6 +323,8 @@ const messages = {

loginAsPublic: 'Logging in as public',
incorrectCredentials: 'Incorrect username or password.',
incorrectOneTimePassword: 'Incorrect one-time password.',
twoFactorAuthenticationEnabled: 'Two factor authentication enabled.',
clearedCredentials: 'Cleared login credentials.',

publishFail: "Couldn't publish package: $0",
Expand Down
11 changes: 10 additions & 1 deletion src/util/request-manager.js
Expand Up @@ -8,7 +8,7 @@ import invariant from 'invariant';
import RequestCaptureHar from 'request-capture-har';

import type {Reporter} from '../reporters/index.js';
import {MessageError, ResponseError} from '../errors.js';
import {MessageError, ResponseError, OneTimePasswordError} from '../errors.js';
import BlockingQueue from './blocking-queue.js';
import * as constants from '../constants.js';
import * as network from './network.js';
Expand Down Expand Up @@ -426,6 +426,15 @@ export default class RequestManager {
}
}

if (res.statusCode === 401 && res.headers['www-authenticate']) {
const authMethods = res.headers['www-authenticate'].split(/,\s*/).map(s => s.toLowerCase());

if (authMethods.indexOf('otp') !== -1) {
reject(new OneTimePasswordError());
return;
}
}

if (body && typeof body.error === 'string') {
reject(new Error(body.error));
return;
Expand Down