Skip to content

Commit

Permalink
feat(auth): Support two factor authentication for NPM accounts (#6555)
Browse files Browse the repository at this point in the history
* feat(auth): Support two factor authentication for npm accounts

Fix #4904

* Add basic tests

* Rename OneTimePasswordRequiredError to OneTimePasswordError

Cause it's also thrown when one-time password is invalid.

* Remove misleading config parameter from getOneTimePassword

* Don't reimplement setOtp in npm-registry.js tests

* Update CHANGELOG.md
  • Loading branch information
neonowy authored and arcanis committed Oct 22, 2018
1 parent 5876b30 commit b8a565e
Show file tree
Hide file tree
Showing 11 changed files with 135 additions and 13 deletions.
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

0 comments on commit b8a565e

Please sign in to comment.