diff --git a/src/server/routes/admin.ts b/src/server/routes/admin.ts index 0de025e..f991ec0 100644 --- a/src/server/routes/admin.ts +++ b/src/server/routes/admin.ts @@ -64,8 +64,8 @@ router.use( res: express.Response, next: express.NextFunction ) => { - const user = await getUser(req); try { + const user = await getUser(req); // only admins are allowed here isOwnerOrAdmin([], user); next(); diff --git a/src/server/routes/gist-private.ts b/src/server/routes/gist-private.ts index 70d2cef..364932a 100644 --- a/src/server/routes/gist-private.ts +++ b/src/server/routes/gist-private.ts @@ -102,8 +102,8 @@ router.delete( router.get( "/source/:gistId", async (req: express.Request, res: express.Response) => { - const user = await getUser(req); try { + const user = await getUser(req); const gist = new Gist( new AnonymizedGistModel({ owner: user.id, @@ -232,10 +232,9 @@ router.post( // add gist router.post("/", async (req: express.Request, res: express.Response) => { - const user = await getUser(req); const gistUpdate = req.body; - try { + const user = await getUser(req); validateNewGist(gistUpdate); const gist = new Gist( diff --git a/src/server/routes/pullRequest-private.ts b/src/server/routes/pullRequest-private.ts index 17f94d6..6fb7dd3 100644 --- a/src/server/routes/pullRequest-private.ts +++ b/src/server/routes/pullRequest-private.ts @@ -103,8 +103,8 @@ router.delete( router.get( "/:owner/:repository/:pullRequestId", async (req: express.Request, res: express.Response) => { - const user = await getUser(req); try { + const user = await getUser(req); const pullRequest = new PullRequest( new AnonymizedPullRequestModel({ owner: user.id, @@ -249,10 +249,9 @@ router.post( // add pullRequest router.post("/", async (req: express.Request, res: express.Response) => { - const user = await getUser(req); const pullRequestUpdate = req.body; - try { + const user = await getUser(req); validateNewPullRequest(pullRequestUpdate); const pullRequest = new PullRequest( diff --git a/src/server/routes/repository-private.ts b/src/server/routes/repository-private.ts index c1b3138..04c6946 100644 --- a/src/server/routes/repository-private.ts +++ b/src/server/routes/repository-private.ts @@ -66,8 +66,8 @@ async function getTokenForAdmin(user: User, req: express.Request) { // claim a repository router.post("/claim", async (req: express.Request, res: express.Response) => { - const user = await getUser(req); try { + const user = await getUser(req); if (!req.body.repoId) { throw new AnonymousError("repoId_not_defined", { object: req.body, @@ -254,12 +254,12 @@ router.delete( router.get( "/:owner/:repo/", async (req: express.Request, res: express.Response) => { - const user = await getUser(req); - let token = user.accessToken; - if (user.isAdmin) { - token = (await getTokenForAdmin(user, req)) || token; - } try { + const user = await getUser(req); + let token = user.accessToken; + if (user.isAdmin) { + token = (await getTokenForAdmin(user, req)) || token; + } const repo = await getRepositoryFromGitHub({ owner: req.params.owner, repo: req.params.repo, @@ -277,12 +277,12 @@ router.get( router.get( "/:owner/:repo/branches", async (req: express.Request, res: express.Response) => { - const user = await getUser(req); - let token = user.accessToken; - if (user.isAdmin) { - token = (await getTokenForAdmin(user, req)) || token; - } try { + const user = await getUser(req); + let token = user.accessToken; + if (user.isAdmin) { + token = (await getTokenForAdmin(user, req)) || token; + } const repository = await getRepositoryFromGitHub({ accessToken: token, owner: req.params.owner, @@ -621,10 +621,9 @@ router.post( // add repository router.post("/", async (req: express.Request, res: express.Response) => { - const user = await getUser(req); const repoUpdate = req.body; - try { + const user = await getUser(req); try { await db.getRepository(repoUpdate.repoId); throw new AnonymousError("repoId_already_used", { diff --git a/test/production-regressions.test.js b/test/production-regressions.test.js index b713825..0ae65a7 100644 --- a/test/production-regressions.test.js +++ b/test/production-regressions.test.js @@ -96,6 +96,19 @@ describe("production regressions", function () { const error = await new Promise(resolve => passport._strategy("github")._verify("token", "", { id: "new-id", username: "recycled" }, resolve)); expect(error.message).to.equal("not_connected"); }); + + for (const name of ["repository-private", "gist-private", "pullRequest-private"]) { + it(`handles rejected authentication in ${name} create routes`, async function () { + const utils = require("../src/server/routes/route-utils"); + const router = require(`../src/server/routes/${name}`).default; + const failure = new Error("banned"); let handled; + stub(utils, "getUser", async () => { throw failure; }); + stub(utils, "handleError", error => { handled = error; }); + const route = router.stack.find(x => x.route?.path === "/" && x.route.methods.post).route; + await route.stack[0].handle({ body: {} }, {}); + expect(handled).to.equal(failure); + }); + } it("checks GitHub authorization before returning shared cached metadata", async function () { const CachedRepoModel = require("../src/core/model/repositories/repositories.model").default; stub(db, "isConnected", true);