Skip to content

Commit

Permalink
fix: If route.isAllowed throws, return 403 instead of 500
Browse files Browse the repository at this point in the history
  • Loading branch information
andreidmt committed Feb 23, 2020
1 parent 531ec90 commit a9d4e39
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 5 deletions.
10 changes: 7 additions & 3 deletions src/plugins/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,20 @@ export default {
}

return (
Promise.resolve(route.isAllowed(req))
Promise.resolve()
.then(() => route.isAllowed(req))
.catch(error => {
debug(`isAllowed for ${route.method}:${route.path} threw`, error)

return false
})
.then(isAllowed => {
if (!isAllowed) {
throw new AuthorizationError("Not allowed to access resource", {
method: route.method,
path: route.path,
})
}

return null
})
// Route action logic
.then(() => route.action(req))
Expand Down
28 changes: 26 additions & 2 deletions tests/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe("blocks :: init with defaults", async assert => {
require("./routes/with-schema.route"),
require("./routes/no-allow.route"),
require("./routes/dont-allow.route"),
require("./routes/allow-throws.route"),
require("./routes/return-undefined.route"),
require("./routes/upload.route"),
],
Expand All @@ -31,10 +32,10 @@ describe("blocks :: init with defaults", async assert => {
})

assert({
given: "6 custom routes",
given: "7 custom routes",
should: "load default /ping and all custom",
actual: plugins.Router.count(),
expected: 7,
expected: 8,
})

assert({
Expand Down Expand Up @@ -121,6 +122,29 @@ describe("blocks :: init with defaults", async assert => {
},
})

assert({
given: "route isAllowed throws error",
should: "return 403",
actual: await GET(`${API_URL}/allow-throws`).catch(
({ status, body }) => ({
status,
body,
})
),
expected: {
status: 403,
body: {
error: "AuthorizationError",
code: 403,
message: "Not allowed to access resource",
details: {
method: "GET",
path: "/allow-throws",
},
},
},
})

assert({
given: "accept app/json on route that returns undefined",
should: "return empty JSON object",
Expand Down
32 changes: 32 additions & 0 deletions tests/routes/allow-throws.route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const debug = require("debug")("Blocks:CustomRoute")

module.exports = {
method: "GET",
path: "/allow-throws",

/**
* Permission checking, if allowed:
* -> continue to action
* -> otherwise return 403
*
* @param {Object} plugins Plugins
* @param {Object} req Node request
*
* @return {boolean}
*/
isAllowed: () => () => {
throw new Error("trololo")
},

/**
* After schema validation and permission checking, do route logic
*
* @param {Object} plugins Plugins
* @param {Object} req Node request
*
* @return {mixed}
*/
action: () => () => ({
ping: "pong",
}),
}

0 comments on commit a9d4e39

Please sign in to comment.