From 36798cfb5fe7343b3e3ffd49ed68efc543b4fcff Mon Sep 17 00:00:00 2001 From: Zaid Abdulla Date: Sat, 24 Oct 2020 10:24:19 -0700 Subject: [PATCH] Fix confusing 404 for unauthorized requests --- lib/ui/api.ts | 58 +++++++++++++++++++++++++-------------------------- ui/store.ts | 4 ++++ 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/lib/ui/api.ts b/lib/ui/api.ts index 16b0285..250881a 100644 --- a/lib/ui/api.ts +++ b/lib/ui/api.ts @@ -86,7 +86,7 @@ router.get(`/devices/:id.csv`, async (ctx) => { if (!authorizer.hasAccess("devices", 2)) { logUnauthorizedWarning(log); - return void (ctx.status = 404); + return void (ctx.status = 403); } const res = await db.query("devices", filter); @@ -127,7 +127,7 @@ router.get(`/devices/:id.csv`, async (ctx) => { }); for (const [resource, flags] of Object.entries(resources)) { - router.head(`/${resource}`, async (ctx, next) => { + router.head(`/${resource}`, async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; let filter: Expression = authorizer.getFilter(resource, 1); if (ctx.request.query.filter) @@ -142,7 +142,7 @@ for (const [resource, flags] of Object.entries(resources)) { if (!authorizer.hasAccess(resource, 1)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } // Exclude temporary tasks and faults @@ -163,7 +163,7 @@ for (const [resource, flags] of Object.entries(resources)) { logger.accessInfo(log); }); - router.get(`/${resource}`, async (ctx, next) => { + router.get(`/${resource}`, async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const options: QueryOptions = {}; let filter: Expression = authorizer.getFilter(resource, 2); @@ -191,7 +191,7 @@ for (const [resource, flags] of Object.entries(resources)) { if (!authorizer.hasAccess(resource, 2)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } // Exclude temporary tasks and faults @@ -217,7 +217,7 @@ for (const [resource, flags] of Object.entries(resources)) { }); // CSV download - router.get(`/${resource}.csv`, async (ctx, next) => { + router.get(`/${resource}.csv`, async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const options: QueryOptions = { projection: {} }; let filter: Expression = authorizer.getFilter(resource, 2); @@ -239,7 +239,7 @@ for (const [resource, flags] of Object.entries(resources)) { if (!authorizer.hasAccess(resource, 2)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const columns: Record = JSON.parse( @@ -308,7 +308,7 @@ for (const [resource, flags] of Object.entries(resources)) { logger.accessInfo(log); }); - router.head(`/${resource}/:id`, async (ctx, next) => { + router.head(`/${resource}/:id`, async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const log = { message: `Count ${resource}`, @@ -323,18 +323,18 @@ for (const [resource, flags] of Object.entries(resources)) { ]); if (!authorizer.hasAccess(resource, 2)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const res = await db.query(resource, filter); - if (!res.length) return void next(); + if (!res.length) return void (ctx.status = 404); logger.accessInfo(log); ctx.body = ""; }); - router.get(`/${resource}/:id`, async (ctx, next) => { + router.get(`/${resource}/:id`, async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const log = { message: `Query ${resource}`, @@ -349,19 +349,19 @@ for (const [resource, flags] of Object.entries(resources)) { ]); if (!authorizer.hasAccess(resource, 2)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const res = await db.query(resource, filter); - if (!res.length) return void next(); + if (!res.length) return void (ctx.status = 404); logger.accessInfo(log); ctx.body = res[0]; }); if (flags & RESOURCE_DELETE) { - router.delete(`/${resource}/:id`, async (ctx, next) => { + router.delete(`/${resource}/:id`, async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const log = { message: `Delete ${resource}`, @@ -376,10 +376,10 @@ for (const [resource, flags] of Object.entries(resources)) { ]); if (!authorizer.hasAccess(resource, 3)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const res = await db.query(resource, filter); - if (!res.length) return void next(); + if (!res.length) return void (ctx.status = 404); const validate = authorizer.getValidator(resource, res[0]); if (!validate("delete")) { @@ -396,7 +396,7 @@ for (const [resource, flags] of Object.entries(resources)) { } if (flags & RESOURCE_PUT) { - router.put(`/${resource}/:id`, async (ctx, next) => { + router.put(`/${resource}/:id`, async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const id = ctx.params.id; @@ -408,7 +408,7 @@ for (const [resource, flags] of Object.entries(resources)) { if (!authorizer.hasAccess(resource, 3)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const obj = ctx.request.body; @@ -435,7 +435,7 @@ for (const [resource, flags] of Object.entries(resources)) { } } -router.put("/files/:id", async (ctx, next) => { +router.put("/files/:id", async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const resource = "files"; const id = ctx.params.id; @@ -449,7 +449,7 @@ router.put("/files/:id", async (ctx, next) => { if (!authorizer.hasAccess(resource, 3)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const metadata = { @@ -478,7 +478,7 @@ router.put("/files/:id", async (ctx, next) => { ctx.body = ""; }); -router.post("/devices/:id/tasks", async (ctx, next) => { +router.post("/devices/:id/tasks", async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const log = { message: "Commit tasks", @@ -494,10 +494,10 @@ router.post("/devices/:id/tasks", async (ctx, next) => { ]); if (!authorizer.hasAccess("devices", 3)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const devices = await db.query("devices", filter); - if (!devices.length) return void next(); + if (!devices.length) return void (ctx.status = 404); const device = devices[0]; const validate = authorizer.getValidator("devices", device); @@ -546,7 +546,7 @@ router.post("/devices/:id/tasks", async (ctx, next) => { ctx.body = res.tasks; }); -router.post("/devices/:id/tags", async (ctx, next) => { +router.post("/devices/:id/tags", async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const log = { message: "Update tags", @@ -562,10 +562,10 @@ router.post("/devices/:id/tags", async (ctx, next) => { ]); if (!authorizer.hasAccess("devices", 3)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const res = await db.query("devices", filter); - if (!res.length) return void next(); + if (!res.length) return void (ctx.status = 404); const validate = authorizer.getValidator("devices", res[0]); if (!validate("tags", ctx.request.body)) { @@ -601,7 +601,7 @@ router.get("/ping/:host", async (ctx) => { }); }); -router.put("/users/:id/password", async (ctx, next) => { +router.put("/users/:id/password", async (ctx) => { const authorizer: Authorizer = ctx.state.authorizer; const username = ctx.params.id; const log = { @@ -626,7 +626,7 @@ router.put("/users/:id/password", async (ctx, next) => { } } else if (!authorizer.hasAccess("users", 3)) { logUnauthorizedWarning(log); - return void next(); + return void (ctx.status = 403); } const filter = and(authorizer.getFilter("users", 3), [ @@ -635,7 +635,7 @@ router.put("/users/:id/password", async (ctx, next) => { username, ]); const res = await db.query("users", filter); - if (!res.length) return void next(); + if (!res.length) return void (ctx.status = 404); const newPassword = ctx.request.body.newPassword; if (ctx.state.user) { diff --git a/ui/store.ts b/ui/store.ts index aac511b..fa2ee4e 100644 --- a/ui/store.ts +++ b/ui/store.ts @@ -173,6 +173,7 @@ export async function xhrRequest( // https://mithril.js.org/request.html#error-handling if (xhr.status !== 304 && Math.floor(xhr.status / 100) !== 2) { + if (xhr.status === 403) throw new Error("Not authorized"); const err = new Error(); err["message"] = xhr.status === 0 @@ -385,6 +386,7 @@ function _fulfill( filter: memoizedStringify(filter), }), extract: (xhr) => { + if (xhr.status === 403) throw new Error("Not authorized"); if (!xhr.status) { throw new Error("Server is unrachable"); } else if (xhr.status !== 200) { @@ -605,6 +607,7 @@ export function postTasks( url: `api/devices/${encodeURIComponent(deviceId)}/tasks`, body: tasks, extract: (xhr) => { + if (xhr.status === 403) throw new Error("Not authorized"); if (xhr.status !== 200) throw new Error(xhr.response); const connectionRequestStatus = xhr.getResponseHeader( "Connection-Request" @@ -675,6 +678,7 @@ export function resourceExists(resource: string, id: string): Promise { filter: memoizedStringify(filter), }), extract: (xhr) => { + if (xhr.status === 403) throw new Error("Not authorized"); if (!xhr.status) throw new Error("Server is unrachable"); else if (xhr.status !== 200) throw new Error(`Unexpected response status code ${xhr.status}`);