Skip to content

Commit

Permalink
fix: fixing repo related publish errors (#46)
Browse files Browse the repository at this point in the history
  • Loading branch information
@soldair @bcoe
soldair authored and bcoe committed Jan 15, 2020
1 parent 45fd001 commit d9d7049
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 116 deletions.
148 changes: 38 additions & 110 deletions src/lib/write-package.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,17 @@ export const writePackage = async (
const pubKey = await writePackage.datastore.getPublishKey(token + '');

if (!pubKey) {
res.statusMessage = 'token not found';
res.status(401);
const ret = {error: '{"error":"publish key not found"}', statusCode: 401};
res.end(ret.error);
return ret;
return respondWithError(res, 'publish key not found', 401);
}

if (pubKey.expiration && pubKey.expiration <= Date.now()) {
res.statusMessage = 'token expired';
res.status(401);
const ret = {error: '{"error":"publish key expired"}', statusCode: 401};
res.end(ret.error);
return ret;
return respondWithError(res, 'publish key expired', 401);
}

// get the client github user token with pubKey.username
const user = await writePackage.datastore.getUser(pubKey.username);
if (!user) {
res.status(401);
const ret = {
error: '{"error":"publish token unauthenticated"}',
statusCode: 401,
};
res.end(ret.error);
return ret;
return respondWithError(res, 'publish token unauthenticated', 401);
}

console.info(
Expand All @@ -65,18 +51,12 @@ export const writePackage = async (

if (pubKey.package && pubKey.package !== packageName) {
console.info('401. token cannot publish this package ' + packageName);
res.statusMessage = 'token cannot publish this package';
res.status(401);
const ret = {
error: formatError(`
This token cannot publish npm package ${packageName} you\'ll need to
npm login --registry https://wombat-dressing-room.appspot.com
again to publish this package.
`),
statusCode: 401,
};
res.end(ret.error);
return ret;
const msg = `
This token cannot publish npm package ${packageName} you\'ll need to
npm login --registry ${config.userRegistryUrl}
again to publish this package.
`;
return respondWithError(res, msg, 401);
}

// fetch existing packument
Expand All @@ -98,13 +78,8 @@ export const writePackage = async (
// not all packages have a latest dist-tag
} catch (e) {
console.info('got ' + e + ' parsing publish');
res.status(401);
const ret = {
error: '{"error":"malformed json package document in publish"}',
statusCode: 401,
};
res.end('{"error":"malformed json package document in publish"}');
return ret;
const msg = 'malformed json package document in publish';
return respondWithError(res, msg, 400);
}
} else {
// the package already exists!
Expand All @@ -115,32 +90,15 @@ export const writePackage = async (
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);
const ret = {
error: formatError(
'not supported yet. package is rather strange. its not new and has no latest version'
),
statusCode: 500,
};
res.end(ret.error);
return ret;
const msg =
'not supported yet. package is rather strange. its not new and has no latest version';
return respondWithError(res, msg, 500);
}

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

const ret = {
error: formatError(
'in order to publish the latest version must have a repository ' +
user.name +
' can access.'
),
statusCode: 400,
};
res.end(ret.error);
return ret;
const msg = `in order to publish the latest version must have a repository ${user.name} can access.`;
return respondWithError(res, msg, 400);
}

console.info('latest repo ', latest.repository);
Expand All @@ -150,64 +108,31 @@ export const writePackage = async (
// make sure publish user has permission to publish the package
// get the github repository from packument
if (!repo) {
res.status(400);
const ret = {
error: formatError(
'in order to publish the latest version must have a repository on github'
),
statusCode: 400,
};
res.end(ret.error);
return ret;
console.info('failed to find repository in latest.repository field.');
const msg =
'In order to publish through wombat the latest version on npm must have a repository pointing to github';
return respondWithError(res, msg, 400);
}

let repoResp = null;
try {
repoResp = await github.getRepo(repo.name, user.token);
} catch (e) {
const ret = {
error: formatError(
'respository ' +
repo.url +
' doesnt exist or ' +
user.name +
' doesnt have access'
),
statusCode: 400,
};
res.end(ret.error);
return ret;
console.info('failed to get repo response for ' + repo.name + ' ' + e);
const msg = `repository ${repo.url} doesn't exist or ${user.name} doesn't have access.`;
return respondWithError(res, msg, 400);
}

if (!repoResp) {
res.status(404);
const ret = {
error: formatError(
'in order to publish the latest version must have a repository ' +
user.name +
" can't see it"
),
statusCode: 404,
};
res.end(ret.error);
return ret;
const msg = `in order to publish the latest version must have a repository ${user.name} can't see it`;
return respondWithError(res, msg, 400);
}

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

if (!(repoResp.permissions.push || repoResp.permissions.admin)) {
res.status(401);
const ret = {
error: formatError(
user.name +
' cannot push repo ' +
repo.url +
'. push permission required to publish.'
),
statusCode: 401,
};
res.end(ret.error);
return ret;
const msg = `${user.name} cannot push repo ${repo.url}. push permission required to publish.`;
return respondWithError(res, msg, 400);
}

// If the publication key has been configured with GitHub releases as a
Expand All @@ -226,14 +151,7 @@ export const writePackage = async (
res
);
} catch (e) {
res.statusCode = e.statusCode;
res.statusMessage = e.statusMessage;
const ret = {
error: JSON.stringify({error: e.statusMessage}),
statusCode: e.statusCode,
};
res.end(ret.error);
return ret;
return respondWithError(res, e.statusMessage, e.statusCode);
}
}

Expand Down Expand Up @@ -340,6 +258,16 @@ async function enforceMatchingRelease(
}
}

function respondWithError(res: Response, message: string, code = 400) {
res.status(code || 401);
const ret = {
error: formatError(message),
statusCode: code,
};
res.json(ret);
return ret;
}

// the npm client will print non json errors to the screen in publish.
// this is a really great way to give detailed error messages
const formatError = (message: string) => {
Expand Down
20 changes: 14 additions & 6 deletions test/lib/write-package.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ import {writePackage, WriteResponse} from '../../src/lib/write-package';

nock.disableNetConnect();

function mockResponse() {
return {
status: (code: number) => {},
end: () => {},
json: () => {},
} as Response;
}

// 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 = () => {};
Expand All @@ -23,7 +31,7 @@ describe('writePackage', () => {
},
});
const req = {headers: {authorization: 'token: abc123'}} as Request;
const res = {status: (code: number) => {}, end: () => {}} as Response;
const res = mockResponse();
const ret = await writePackage('@soldair/foo', req, res);
expect(ret.statusCode).to.equal(401);
expect(ret.error).to.match(/publish key not found/);
Expand Down Expand Up @@ -52,7 +60,7 @@ describe('writePackage', () => {
.addVersion('1.0.0')
.packument()
);
const res = {status: (code: number) => {}, end: () => {}} as Response;
const res = mockResponse();

// A 404 while fetching the packument indicates that the package
// has not yet been created:
Expand Down Expand Up @@ -100,7 +108,7 @@ describe('writePackage', () => {
.addVersion('1.0.0', 'https://github.com/foo/bar')
.packument()
);
const res = {status: (code: number) => {}, end: () => {}} as Response;
const res = mockResponse();

// A 404 while fetching the packument indicates that the package
// has not yet been created:
Expand Down Expand Up @@ -156,7 +164,7 @@ describe('writePackage', () => {
.addVersion('1.0.0', 'https://github.com/foo/bar')
.packument()
);
const res = {status: (code: number) => {}, end: () => {}} as Response;
const res = mockResponse();

const npmRequest = nock('https://registry.npmjs.org')
.get('/@soldair%2ffoo')
Expand Down Expand Up @@ -213,7 +221,7 @@ describe('writePackage', () => {
{authorization: 'token: abc123'},
'0.2.3'
);
const res = {status: (code: number) => {}, end: () => {}} as Response;
const res = mockResponse();

// A 404 while fetching the packument indicates that the package
// has not yet been created:
Expand Down Expand Up @@ -271,7 +279,7 @@ describe('writePackage', () => {
.addVersion('1.0.0', 'https://github.com/foo/bar')
.packument()
);
const res = {status: (code: number) => {}, end: () => {}} as Response;
const res = mockResponse();

const npmRequest = nock('https://registry.npmjs.org')
.get('/@soldair%2ffoo')
Expand Down

0 comments on commit d9d7049

Please sign in to comment.