From b856f8da944be8287590206b34cfb92b2548931d Mon Sep 17 00:00:00 2001 From: bcoe Date: Thu, 19 Nov 2020 14:47:12 -0800 Subject: [PATCH 1/2] feat: support monorepos with release-backed tokens Allows release-backed tokens to be generated with a monorepo option if monorepo is enabled, release-backed tokens will look for matching releases that contain the package-name suffix, e.g., yargs-v1.0.0 sql-v1.0.2, rather than just v1.0.0, v1.0.2. --- src/lib/datastore.ts | 5 +- src/lib/github.ts | 53 ++++++++++-------- src/lib/write-package.ts | 13 +++-- src/server.ts | 3 +- test/lib/github.ts | 46 +++++++++++++++- test/lib/write-package.ts | 111 +++++++++++++++++++++++++++----------- views/token-settings.hbs | 6 +++ 7 files changed, 177 insertions(+), 60 deletions(-) diff --git a/src/lib/datastore.ts b/src/lib/datastore.ts index b54e742..0017773 100644 --- a/src/lib/datastore.ts +++ b/src/lib/datastore.ts @@ -115,7 +115,8 @@ export const savePublishKey = async ( publishKey: string, packageName?: string, expiration?: number, - releaseAs2FA?: boolean + releaseAs2FA?: boolean, + monorepo?: boolean ): Promise<{}> => { if (!config.loginEnabled) { return Promise.reject(new Error('disabled on this server.')); @@ -135,6 +136,7 @@ export const savePublishKey = async ( package: packageName, expiration, releaseAs2FA, + monorepo, }, }); }; @@ -242,6 +244,7 @@ export interface PublishKey { package?: string; expiration?: number; releaseAs2FA?: boolean; + monorepo?: boolean; } interface UserMain { diff --git a/src/lib/github.ts b/src/lib/github.ts index 35fd789..d76c0d3 100644 --- a/src/lib/github.ts +++ b/src/lib/github.ts @@ -55,38 +55,49 @@ export const getRepo = (name: string, token: string): Promise => { /** * https://developer.github.com/v3/repos/releases/#list-releases-for-a-repository * @param name repository name including username. ex: node/node or bcoe/yargs + * @param prefix if present, it is required that the release tags include the + * prefix, e.g., yargs-v1.0.0. This allows a token to be created that works + * for a monorepo. * @param token * @param tag release tag to fetch. * * @returns string[] tag names of releases. */ -export const getRelease = ( +export const getRelease = async ( name: string, token: string, - tag: string + tag: string, + prefix?: string ): Promise => { const client = gh.client(token, clientOptions); - return new Promise((resolve, reject) => { - client.get( - `/repos/${name}/tags`, - {per_page: 100}, - (err: Error, code: number, resp: [{name: string}]) => { - if (err) { - return reject(err); - } else if (code !== 200) - return reject(new Error(`unexpected http code = ${code}`)); - else if ( - !resp.find(item => { - return item.name === tag; - }) - ) { - return reject(new Error('not found')); - } else { - return resolve(tag); + // We check up to 600 of the most recent tags for a matching release, + // we use a large page size to allow for monorepos with 100s of tags: + const maxPagination = 6; + for (let page = 1; page < maxPagination; page++) { + const tags: [{name: string}] = await new Promise((resolve, reject) => { + client.get( + `/repos/${name}/tags`, + {per_page: 100, page: page}, + (err: Error, code: number, resp: [{name: string}]) => { + if (err) { + return reject(err); + } else if (code !== 200) { + return reject(new Error(`unexpected http code = ${code}`)); + } else { + resolve(resp); + } } + ); + }); + for (const item of tags) { + if (!prefix && item.name === tag) { + return tag; + } else if (item.name === `${prefix}-${tag}`) { + return tag; } - ); - }); + } + } + throw new Error('not found'); }; /** diff --git a/src/lib/write-package.ts b/src/lib/write-package.ts index 3ab7013..3add0b7 100644 --- a/src/lib/write-package.ts +++ b/src/lib/write-package.ts @@ -171,7 +171,8 @@ export const writePackage = async ( repo.name, user.token, newPackage ? undefined : doc, - drainedBody + drainedBody, + pubKey.monorepo ); } catch (e) { return respondWithError(res, e.statusMessage, e.statusCode); @@ -231,7 +232,8 @@ async function enforceMatchingRelease( repoName: string, token: string, lastPackument: Packument | undefined, - drainedBody: Buffer + drainedBody: Buffer, + monorepo?: boolean ) { try { const maybePackument = JSON.parse(drainedBody + ''); @@ -278,7 +280,12 @@ async function enforceMatchingRelease( } } try { - await github.getRelease(repoName, token, `v${newVersion}`); + let prefix; + if (monorepo) { + const splitName = newPackument.name.split('/'); + prefix = splitName.length === 1 ? splitName[0] : splitName[1]; + } + await github.getRelease(repoName, token, `v${newVersion}`, prefix); } catch (err) { const msg = `matching release v${newVersion} not found for ${repoName}`; throw new WombatServerError(msg, 400); diff --git a/src/server.ts b/src/server.ts index 511bdf3..6f6df0f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -371,7 +371,8 @@ app.get( handoff.value, req.query.package ? (req.query.package + '').trim() : undefined, ttl, - releaseAs2FA + releaseAs2FA, + req.query.monorepo === 'on' ? true : false ), datastore.completeHandoffKey(req.query.ott + ''), ]); diff --git a/test/lib/github.ts b/test/lib/github.ts index 868c8cd..2544b2c 100644 --- a/test/lib/github.ts +++ b/test/lib/github.ts @@ -25,7 +25,7 @@ describe('github', () => { describe('getLatestRelease', () => { it('returns latest release from GitHub', async () => { const request = nock('https://api.github.com') - .get('/repos/bcoe/test/tags?per_page=100') + .get('/repos/bcoe/test/tags?per_page=100&page=1') .reply(200, [{name: 'v1.0.2'}]); const latest = await github.getRelease('bcoe/test', 'abc123', 'v1.0.2'); @@ -35,7 +35,7 @@ describe('github', () => { it('bubbles error appropriately', async () => { const request = nock('https://api.github.com') - .get('/repos/bcoe/test/tags?per_page=100') + .get('/repos/bcoe/test/tags?per_page=100&page=1') .reply(404); let err: Error | undefined = undefined; try { @@ -49,5 +49,47 @@ describe('github', () => { } request.done(); }); + + it('does not return latest release without prefix, when prefix used', async () => { + const request = nock('https://api.github.com') + .get('/repos/bcoe/test/tags?per_page=100&page=1') + .reply(200, [{name: 'v1.0.2'}]) + .get('/repos/bcoe/test/tags?per_page=100&page=2') + .reply(200, [{name: 'v1.0.3'}]) + .get('/repos/bcoe/test/tags?per_page=100&page=3') + .reply(200, [{name: 'v1.0.4'}]) + .get('/repos/bcoe/test/tags?per_page=100&page=4') + .reply(200, [{name: 'v1.0.5'}]) + .get('/repos/bcoe/test/tags?per_page=100&page=5') + .reply(200, [{name: 'v1.0.6'}]); + + let err: Error; + try { + await github.getRelease('bcoe/test', 'abc123', 'v1.0.2', 'foo'); + } catch (_err) { + err = _err; + } + expect(err!.message).to.equal('not found'); + request.done(); + }); + + it('returns latest release matching prefix', async () => { + const request = nock('https://api.github.com') + .get('/repos/bcoe/test/tags?per_page=100&page=1') + .reply(200, [{name: 'v1.0.3'}]) + .get('/repos/bcoe/test/tags?per_page=100&page=2') + .reply(200, [{name: 'v1.0.4'}]) + .get('/repos/bcoe/test/tags?per_page=100&page=3') + .reply(200, [{name: 'foo-v1.0.2'}]); + + const latest = await github.getRelease( + 'bcoe/test', + 'abc123', + 'v1.0.2', + 'foo' + ); + expect(latest).to.equal('v1.0.2'); + request.done(); + }); }); }); diff --git a/test/lib/write-package.ts b/test/lib/write-package.ts index 7d30f1d..117d43f 100644 --- a/test/lib/write-package.ts +++ b/test/lib/write-package.ts @@ -30,6 +30,7 @@ nock.disableNetConnect(); function mockResponse() { return { + // eslint-disable-next-line @typescript-eslint/no-unused-vars status: (_code: number) => { return; }, @@ -45,7 +46,7 @@ console.info = () => {}; describe('writePackage', () => { it('responds with 401 if publication key not found in datastore', async () => { writePackage.datastore = Object.assign({}, datastore, { - getPublishKey: async (_username: string): Promise => { + getPublishKey: async (): Promise => { return false; }, }); @@ -59,7 +60,7 @@ describe('writePackage', () => { it('responds with 400 if packument has no repository field', async () => { // Fake that there's a releaseAs2FA key in datastore: writePackage.datastore = Object.assign({}, datastore, { - getPublishKey: async (_username: string): Promise => { + getPublishKey: async (): Promise => { return { username: 'bcoe', created: 1578630249529, @@ -67,7 +68,7 @@ describe('writePackage', () => { releaseAs2FA: true, }; }, - getUser: async (_username: string): Promise => { + getUser: async (): Promise => { return {name: 'bcoe', token: 'deadbeef'}; }, }); @@ -95,9 +96,7 @@ describe('writePackage', () => { 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 => { + getPublishKey: async (): Promise => { return { username: 'bcoe', created: 1578630249529, @@ -105,7 +104,7 @@ describe('writePackage', () => { releaseAs2FA: true, }; }, - getUser: async (_username: string): Promise => { + getUser: async (): Promise => { return {name: 'bcoe', token: 'deadbeef'}; }, }); @@ -138,7 +137,7 @@ describe('writePackage', () => { .get('/repos/foo/bar') .reply(200, {permissions: {push: true}}) // most recent release tag on GitHub is v1.0.0 - .get('/repos/foo/bar/tags?per_page=100') + .get('/repos/foo/bar/tags?per_page=100&page=1') .reply(200, [{name: 'v1.0.0'}]); const ret = await writePackage('@soldair/foo', req, res); @@ -151,9 +150,7 @@ describe('writePackage', () => { it('allows a package to be updated', async () => { // Fake that there's a releaseAs2FA key in datastore: writePackage.datastore = Object.assign({}, datastore, { - getPublishKey: async ( - _username: string - ): Promise => { + getPublishKey: async (): Promise => { return { username: 'bcoe', created: 1578630249529, @@ -161,7 +158,7 @@ describe('writePackage', () => { releaseAs2FA: true, }; }, - getUser: async (_username: string): Promise => { + getUser: async (): Promise => { return {name: 'bcoe', token: 'deadbeef'}; }, }); @@ -197,7 +194,7 @@ describe('writePackage', () => { .get('/repos/foo/bar') .reply(200, {permissions: {push: true}}) // most recent release tag on GitHub is v1.0.0 - .get('/repos/foo/bar/tags?per_page=100') + .get('/repos/foo/bar/tags?per_page=100&page=1') .reply(200, [{name: 'v1.0.0'}]); const ret = await writePackage('@soldair/foo', req, res); @@ -210,9 +207,7 @@ describe('writePackage', () => { it('supports publication to next tag', async () => { // Fake that there's a releaseAs2FA key in datastore: writePackage.datastore = Object.assign({}, datastore, { - getPublishKey: async ( - _username: string - ): Promise => { + getPublishKey: async (): Promise => { return { username: 'bcoe', created: 1578630249529, @@ -220,7 +215,7 @@ describe('writePackage', () => { releaseAs2FA: true, }; }, - getUser: async (_username: string): Promise => { + getUser: async (): Promise => { return {name: 'bcoe', token: 'deadbeef'}; }, }); @@ -256,7 +251,7 @@ describe('writePackage', () => { .get('/repos/foo/bar') .reply(200, {permissions: {push: true}}) // most recent release tag on GitHub is v1.0.0 - .get('/repos/foo/bar/tags?per_page=100') + .get('/repos/foo/bar/tags?per_page=100&page=1') .reply(200, [{name: 'v1.0.0'}]); const ret = await writePackage('@soldair/foo', req, res); @@ -269,9 +264,7 @@ describe('writePackage', () => { it("does not allow PUTs that aren't publications, e.g., dist-tag updates", async () => { // Fake that there's a releaseAs2FA key in datastore: writePackage.datastore = Object.assign({}, datastore, { - getPublishKey: async ( - _username: string - ): Promise => { + getPublishKey: async (): Promise => { return { username: 'bcoe', created: 1578630249529, @@ -279,7 +272,7 @@ describe('writePackage', () => { releaseAs2FA: true, }; }, - getUser: async (_username: string): Promise => { + getUser: async (): Promise => { return {name: 'bcoe', token: 'deadbeef'}; }, }); @@ -325,9 +318,7 @@ describe('writePackage', () => { it('rejects publication if no corresponding release found on GitHub', async () => { // Fake that there's a releaseAs2FA key in datastore: writePackage.datastore = Object.assign({}, datastore, { - getPublishKey: async ( - _username: string - ): Promise => { + getPublishKey: async (): Promise => { return { username: 'bcoe', created: 1578630249529, @@ -335,7 +326,7 @@ describe('writePackage', () => { releaseAs2FA: true, }; }, - getUser: async (_username: string): Promise => { + getUser: async (): Promise => { return {name: 'bcoe', token: 'deadbeef'}; }, }); @@ -371,7 +362,7 @@ describe('writePackage', () => { .get('/repos/foo/bar') .reply(200, {permissions: {push: true}}) // most recent release tag on GitHub is v0.1.0 - .get('/repos/foo/bar/tags?per_page=100') + .get('/repos/foo/bar/tags?per_page=100&page=1') .reply(200, [{name: 'v0.1.0'}]); const ret = await writePackage('@soldair/foo', req, res); @@ -384,9 +375,7 @@ describe('writePackage', () => { it('rejects publication if listing tags rerturns non-200', async () => { // Fake that there's a releaseAs2FA key in datastore: writePackage.datastore = Object.assign({}, datastore, { - getPublishKey: async ( - _username: string - ): Promise => { + getPublishKey: async (): Promise => { return { username: 'bcoe', created: 1578630249529, @@ -394,7 +383,7 @@ describe('writePackage', () => { releaseAs2FA: true, }; }, - getUser: async (_username: string): Promise => { + getUser: async (): Promise => { return {name: 'bcoe', token: 'deadbeef'}; }, }); @@ -430,7 +419,7 @@ describe('writePackage', () => { .get('/repos/foo/bar') .reply(200, {permissions: {push: true}}) // most recent release tag on GitHub is v0.1.0 - .get('/repos/foo/bar/tags?per_page=100') + .get('/repos/foo/bar/tags?per_page=100&page=1') .reply(500); const ret = await writePackage('@soldair/foo', req, res); @@ -439,5 +428,63 @@ describe('writePackage', () => { expect(ret.error).to.match(/matching release v1.0.0 not found/); expect(ret.statusCode).to.equal(400); }); + + it('allows package with monorepo token to be updated', async () => { + // Fake that there's a releaseAs2FA key in datastore: + writePackage.datastore = Object.assign({}, datastore, { + getPublishKey: async (): Promise => { + return { + username: 'bcoe', + created: 1578630249529, + value: 'deadbeef', + releaseAs2FA: true, + monorepo: true, + }; + }, + getUser: async (): Promise => { + return {name: 'bcoe', token: 'deadbeef'}; + }, + }); + writePackage.pipeToNpm = ( + req: Request, + res: Response, + drainedBody: false | Buffer, + newPackage: boolean + ): Promise => { + return Promise.resolve({statusCode: 200, newPackage}); + }; + + // Simulate a publication request to the proxy: + const req = writePackageRequest( + {authorization: 'token: abc123'}, + createPackument('@soldair/foo') + .addVersion('1.0.0', 'https://github.com/foo/bar') + .packument() + ); + const res = mockResponse(); + + const npmRequest = nock('https://registry.npmjs.org') + .get('/@soldair%2ffoo') + .reply( + 200, + createPackument('@soldair/foo') + .addVersion('0.1.0', 'https://github.com/foo/bar') + .packument() + ); + + 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/tags?per_page=100&page=1') + .reply(200, [{name: 'foo-v1.0.0'}]); + + const ret = await writePackage('@soldair/foo', req, res); + npmRequest.done(); + githubRequest.done(); + expect(ret.statusCode).to.equal(200); + expect(ret.newPackage).to.equal(false); + }); }); }); diff --git a/views/token-settings.hbs b/views/token-settings.hbs index a0f3111..445e17f 100644 --- a/views/token-settings.hbs +++ b/views/token-settings.hbs @@ -26,6 +26,12 @@
+
+ + +
From 6966cabce3e21e84416c8efb514712a02b035acd Mon Sep 17 00:00:00 2001 From: bcoe Date: Fri, 20 Nov 2020 08:26:50 -0800 Subject: [PATCH 2/2] chore: address code review --- src/lib/github.ts | 12 ++++++++---- src/lib/write-package.ts | 21 +++++++++++++-------- test/lib/github.ts | 10 +++------- test/lib/write-package.ts | 13 +++++++++++-- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/lib/github.ts b/src/lib/github.ts index d76c0d3..a3a5fbc 100644 --- a/src/lib/github.ts +++ b/src/lib/github.ts @@ -68,7 +68,7 @@ export const getRelease = async ( token: string, tag: string, prefix?: string -): Promise => { +): Promise => { const client = gh.client(token, clientOptions); // We check up to 600 of the most recent tags for a matching release, // we use a large page size to allow for monorepos with 100s of tags: @@ -80,9 +80,13 @@ export const getRelease = async ( {per_page: 100, page: page}, (err: Error, code: number, resp: [{name: string}]) => { if (err) { - return reject(err); + return reject(Error(`getRelease: tag = ${tag}`)); } else if (code !== 200) { - return reject(new Error(`unexpected http code = ${code}`)); + return reject( + new Error( + `getRelease: unexpected http code = ${code} tag = ${tag}` + ) + ); } else { resolve(resp); } @@ -97,7 +101,7 @@ export const getRelease = async ( } } } - throw new Error('not found'); + return undefined; }; /** diff --git a/src/lib/write-package.ts b/src/lib/write-package.ts index 3add0b7..94144e8 100644 --- a/src/lib/write-package.ts +++ b/src/lib/write-package.ts @@ -279,18 +279,23 @@ async function enforceMatchingRelease( newVersion = versions[0]; } } - try { - let prefix; - if (monorepo) { - const splitName = newPackument.name.split('/'); - prefix = splitName.length === 1 ? splitName[0] : splitName[1]; - } - await github.getRelease(repoName, token, `v${newVersion}`, prefix); - } catch (err) { + let prefix; + if (monorepo) { + const splitName = newPackument.name.split('/'); + prefix = splitName.length === 1 ? splitName[0] : splitName[1]; + } + const release = await github.getRelease( + repoName, + token, + `v${newVersion}`, + prefix + ); + if (!release) { const msg = `matching release v${newVersion} not found for ${repoName}`; throw new WombatServerError(msg, 400); } } catch (err) { + console.error(err); if (err.statusCode && err.statusMessage) throw err; err.statusCode = 500; err.statusMessage = 'unknown error'; diff --git a/test/lib/github.ts b/test/lib/github.ts index 2544b2c..ebd7238 100644 --- a/test/lib/github.ts +++ b/test/lib/github.ts @@ -63,13 +63,9 @@ describe('github', () => { .get('/repos/bcoe/test/tags?per_page=100&page=5') .reply(200, [{name: 'v1.0.6'}]); - let err: Error; - try { - await github.getRelease('bcoe/test', 'abc123', 'v1.0.2', 'foo'); - } catch (_err) { - err = _err; - } - expect(err!.message).to.equal('not found'); + expect( + await github.getRelease('bcoe/test', 'abc123', 'v1.0.2', 'foo') + ).to.equal(undefined); request.done(); }); diff --git a/test/lib/write-package.ts b/test/lib/write-package.ts index 117d43f..dd93c96 100644 --- a/test/lib/write-package.ts +++ b/test/lib/write-package.ts @@ -42,6 +42,7 @@ function mockResponse() { // 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 = () => {}; +console.error = () => {}; describe('writePackage', () => { it('responds with 401 if publication key not found in datastore', async () => { @@ -363,6 +364,14 @@ describe('writePackage', () => { .reply(200, {permissions: {push: true}}) // most recent release tag on GitHub is v0.1.0 .get('/repos/foo/bar/tags?per_page=100&page=1') + .reply(200, [{name: 'v0.1.0'}]) + .get('/repos/foo/bar/tags?per_page=100&page=2') + .reply(200, [{name: 'v0.1.0'}]) + .get('/repos/foo/bar/tags?per_page=100&page=3') + .reply(200, [{name: 'v0.1.0'}]) + .get('/repos/foo/bar/tags?per_page=100&page=4') + .reply(200, [{name: 'v0.1.0'}]) + .get('/repos/foo/bar/tags?per_page=100&page=5') .reply(200, [{name: 'v0.1.0'}]); const ret = await writePackage('@soldair/foo', req, res); @@ -425,8 +434,8 @@ describe('writePackage', () => { const ret = await writePackage('@soldair/foo', req, res); npmRequest.done(); githubRequest.done(); - expect(ret.error).to.match(/matching release v1.0.0 not found/); - expect(ret.statusCode).to.equal(400); + expect(ret.error).to.match(/unknown error/); + expect(ret.statusCode).to.equal(500); }); it('allows package with monorepo token to be updated', async () => {