Skip to content

Commit

Permalink
fix: logic bug when detecting initial publications (#17)
Browse files Browse the repository at this point in the history
  • Loading branch information
bcoe committed Jan 10, 2020
1 parent 8132fea commit 3a0f328
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 50 deletions.
1 change: 1 addition & 0 deletions .nycrc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"**/scripts",
"**/protos",
"**/test",
"**/*.d.ts",
".jsdoc.js",
"**/.jsdoc.js",
"karma.conf.js",
Expand Down
2 changes: 1 addition & 1 deletion src/lib/datastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ export const deletePublishKey = async (token: string) => {
};


type User = {
export type User = {
// tslint:disable-next-line:no-any
[k: string]: any
}|UserMain;
Expand Down
101 changes: 54 additions & 47 deletions src/lib/write-package.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Packument} from '@npm/types';
import * as express from 'express';
import {Request, Response} from 'express';
import * as request from 'request';

import {config} from '../lib/config';
Expand All @@ -12,15 +12,15 @@ import {newVersions} from './new-versions';
import {findLatest, packument, repoToGithub} from './packument';
import {WombatServerError} from './wombat-server-error';

interface WriteResponse {
export interface WriteResponse {
statusCode: number;
error?: string;
newPackage?: boolean;
}

export const writePackage = async(
packageName: string, req: express.Request,
res: express.Response): Promise<WriteResponse> => {
packageName: string, req: Request,
res: Response): Promise<WriteResponse> => {
// verify authorization.
const auth = req.headers.authorization + '';
const token = auth.split(' ').pop();
Expand Down Expand Up @@ -54,12 +54,12 @@ export const writePackage = async(
return ret;
}

console.log(
console.info(
'attempting to publish package ' + packageName +
' with publish key config ' + pubKey.package);

if (pubKey.package && pubKey.package !== packageName) {
console.log('401. token cannot publish this package ' + packageName);
console.info('401. token cannot publish this package ' + packageName);
res.statusMessage = 'token cannot publish this package';
res.status(401);
const ret = {
Expand All @@ -75,7 +75,7 @@ export const writePackage = async(
}

// fetch existing packument
console.log('fetching ', packageName, 'from npm');
console.info('fetching ', packageName, 'from npm');
let doc = await packument(packageName);

let latest = undefined;
Expand All @@ -92,7 +92,7 @@ export const writePackage = async(
latest = doc.versions[doc['dist-tags'].latest || ''];
// not all packages have a latest dist-tag
} catch (e) {
console.log('got ' + e + ' parsing publish');
console.info('got ' + e + ' parsing publish');
res.status(401);
const ret = {
error: '{"error":"malformed json package document in publish"}',
Expand All @@ -108,7 +108,7 @@ export const writePackage = async(
}

if (!latest) {
console.log('missing latest version for ' + packageName);
console.info('missing latest version for ' + packageName);
// we need to verify that this package has a repo config that points to
// github so users don't lock themselves out.
res.status(500);
Expand All @@ -122,7 +122,7 @@ export const writePackage = async(
}

if (!latest.repository) {
console.log('missing repository in the latest version of ' + packageName);
console.info('missing repository in the latest version of ' + packageName);
res.statusMessage = 'latest npm version missing github repository';
res.statusCode = 400;

Expand All @@ -136,7 +136,7 @@ export const writePackage = async(
return ret;
}

console.log('latest repo ', latest.repository);
console.info('latest repo ', latest.repository);

const repo = repoToGithub(latest.repository);

Expand Down Expand Up @@ -179,7 +179,7 @@ export const writePackage = async(
return ret;
}

console.log('repo response!', repoResp.permissions);
console.info('repo response!', repoResp.permissions);

if (!(repoResp.permissions.push || repoResp.permissions.admin)) {
res.status(401);
Expand All @@ -201,7 +201,8 @@ export const writePackage = async(
drainedBody = drainedBody || await drainRequest(req);
try {
await enforceMatchingRelease(
repo.name, user.token, doc, drainedBody, req, res);
repo.name, user.token, newPackage ? undefined : doc, drainedBody, req,
res);
} catch (e) {
res.statusCode = e.statusCode;
res.statusMessage = e.statusMessage;
Expand All @@ -214,48 +215,54 @@ export const writePackage = async(
}
}

// update auth information to be the publish user.
req.headers.authorization = 'Bearer ' + config.npmToken;
// send 2fa token.
req.headers['npm-otp'] = totpCode(config.totpSecret);
req.headers.host = 'registry.npmjs.org';
const npmreq = request(
'https://registry.npmjs.org' + req.url,
{method: req.method, headers: req.headers});

// if we've buffered the publish request already
if (drainedBody) {
npmreq.pipe(res);
npmreq.write(drainedBody);
// TODO: missing end here? make sure this path is covered.
} else {
req.pipe(npmreq).pipe(res);
}

req.on('error', (e: Error) => {
console.log('oh how strange. request errored', e);
});
npmreq.on('error', (e: Error) => {
console.log('npm request error ', e);
});
res.on('error', (e: Error) => {
console.log('error sending response for npm publish', e);
});

return new Promise((resolve, reject) => {
npmreq.on('response', async (npmres) => {
resolve({statusCode: npmres.statusCode, newPackage});
});
});
return writePackage.pipeToNpm(req, res, drainedBody, newPackage);
};

writePackage.pipeToNpm =
(req: Request, res: Response, drainedBody: false|Buffer,
newPackage: boolean): Promise<WriteResponse> => {
// update auth information to be the publish user.
req.headers.authorization = 'Bearer ' + config.npmToken;
// send 2fa token.
req.headers['npm-otp'] = totpCode(config.totpSecret);
req.headers.host = 'registry.npmjs.org';
const npmreq = request(
'https://registry.npmjs.org' + req.url,
{method: req.method, headers: req.headers});

// if we've buffered the publish request already
if (drainedBody) {
npmreq.pipe(res);
npmreq.write(drainedBody);
// TODO: missing end here? make sure this path is covered.
} else {
req.pipe(npmreq).pipe(res);
}

req.on('error', (e: Error) => {
console.info('oh how strange. request errored', e);
});
npmreq.on('error', (e: Error) => {
console.info('npm request error ', e);
});
res.on('error', (e: Error) => {
console.info('error sending response for npm publish', e);
});

return new Promise((resolve, reject) => {
npmreq.on('response', async (npmres) => {
resolve({statusCode: npmres.statusCode, newPackage});
});
});
};

/*
* Throws an exception if a matching GitHub release cannot be found for the
* packument that is being published to npm.
*/
async function enforceMatchingRelease(
repoName: string, token: string, lastPackument: Packument|undefined,
drainedBody: Buffer, req: express.Request, res: express.Response) {
drainedBody: Buffer, req: Request, res: Response) {
try {
const newPackument = JSON.parse(drainedBody + '') as Packument;
// Some types of updates don't include a full packument, e.g., changing
Expand Down
78 changes: 76 additions & 2 deletions test/lib/write-package.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import {expect} from 'chai';
import {Request, Response} from 'express';
import * as nock from 'nock';

import * as datastore from '../../src/lib/datastore';
import {PublishKey} from '../../src/lib/datastore';
import {writePackage} from '../../src/lib/write-package';
import {PublishKey, User} from '../../src/lib/datastore';
import {writePackage, WriteResponse} from '../../src/lib/write-package';

import request = require('request');

nock.disableNetConnect();

// TODO: rather than silencing info level logging, let's consider moving to
// a logger like winston or bunyan, which is easier to turn off in tests.
console.info = () => {};

describe('writePackage', () => {
it('responds with 401 if publication key not found in datastore',
async () => {
Expand All @@ -21,4 +28,71 @@ describe('writePackage', () => {
expect(ret.statusCode).to.equal(401);
expect(ret.error).to.match(/publish key not found/);
});

describe('releaseAs2FA', () => {
it('appropriately routes initial package publication', async () => {
// Fake that there's a releaseAs2FA key in datastore:
writePackage.datastore = Object.assign({}, datastore, {
getPublishKey: async(username: string): Promise<PublishKey|false> => {
return {
username: 'bcoe',
created: 1578630249529,
value: 'deadbeef',
releaseAs2FA: true
};
},
getUser: async(username: string): Promise<false|User> => {
return {name: 'bcoe', token: 'deadbeef'};
}
});
writePackage.pipeToNpm =
(req: Request, res: Response, drainedBody: false|Buffer,
newPackage: boolean): Promise<WriteResponse> => {
return Promise.resolve({statusCode: 200, newPackage});
};

// Simulate a package publication request:
const req = {
headers: {authorization: 'token: abc123'},
on: (event: 'data'|'end', listener: (buffer?: Buffer) => void) => {
switch (event) {
case 'data':
listener(Buffer.from(JSON.stringify({
versions: {
'1.0.0': {
version: '1.0.0',
repository: 'https://github.com/foo/bar'
}
},
'dist-tags': {'latest': '1.0.0'}
})));
break;
case 'end':
listener();
break;
default:
break;
}
}
} as Request;
const res = {status: (code: number) => {}, end: () => {}} as Response;

// A 404 while fetching the packument indicates that the package
// has not yet been created:
const npmRequest =
nock('https://registry.npmjs.org').get('/@soldair%2ffoo').reply(404);

const githubRequest = nock('https://api.github.com')
// user has push access to repo in package.json
.get('/repos/foo/bar')
.reply(200, {permissions: {push: true}})
// most recent release tag on GitHub is v1.0.0
.get('/repos/foo/bar/releases/latest')
.reply(200, {tag_name: 'v1.0.0'});

const ret = await writePackage('@soldair/foo', req, res);
expect(ret.newPackage).to.equal(true);
expect(ret.statusCode).to.equal(200);
});
});
});

0 comments on commit 3a0f328

Please sign in to comment.