From 306015f0222864f98e1c5aafc4b0b4fac22c3237 Mon Sep 17 00:00:00 2001 From: Zaid Abdulla Date: Fri, 11 Oct 2019 12:41:35 -0700 Subject: [PATCH] Fix permission filter not being applied --- lib/common/authorizer.ts | 27 ++++++++++-- lib/common/expression.ts | 4 +- lib/ui/api.ts | 90 ++++++++++++++++++++++++++++------------ 3 files changed, 89 insertions(+), 32 deletions(-) diff --git a/lib/common/authorizer.ts b/lib/common/authorizer.ts index 170607d..81c0b9f 100644 --- a/lib/common/authorizer.ts +++ b/lib/common/authorizer.ts @@ -18,7 +18,7 @@ */ import { PermissionSet, Expression } from "../types"; -import { evaluate } from "./expression"; +import { evaluate, or } from "./expression"; export default class Authorizer { private permissionSets: PermissionSet[]; @@ -27,14 +27,16 @@ export default class Authorizer { (mutationType, mutation, any) => boolean >; private hasAccessCache: Map; + private getFilterCache: Map; public constructor(permissionSets) { this.permissionSets = permissionSets; this.validatorCache = new WeakMap(); this.hasAccessCache = new Map(); + this.getFilterCache = new Map(); } - public hasAccess(resourceType, access): boolean { + public hasAccess(resourceType: string, access: number): boolean { const cacheKey = `${resourceType}-${access}`; if (this.hasAccessCache.has(cacheKey)) return this.hasAccessCache.get(cacheKey); @@ -55,10 +57,29 @@ export default class Authorizer { return has; } + public getFilter(resourceType: string, access: number): Expression { + const cacheKey = `${resourceType}-${access}`; + if (this.getFilterCache.has(cacheKey)) + return this.getFilterCache.get(cacheKey); + + let filter: Expression = null; + for (const permissionSet of this.permissionSets) { + for (const perm of permissionSet) { + if (perm[resourceType]) { + if (perm[resourceType].access >= access) + filter = or(filter, perm[resourceType].filter); + } + } + } + + this.getFilterCache.set(cacheKey, filter); + return filter; + } + public getValidator( resourceType, resource - ): (mutationType: string, mutation: any, args: any) => boolean { + ): (mutationType: string, mutation?: any, args?: any) => boolean { if (this.validatorCache.has(resource)) return this.validatorCache.get(resource); diff --git a/lib/common/expression.ts b/lib/common/expression.ts index efa12af..2f30cbd 100644 --- a/lib/common/expression.ts +++ b/lib/common/expression.ts @@ -277,7 +277,7 @@ export async function evaluateAsync( }); } -export function and(exp1, exp2): Expression { +export function and(exp1: Expression, exp2: Expression): Expression { if (!isArray(exp1)) return exp1 ? exp2 : exp1; if (!isArray(exp2)) return exp2 ? exp1 : exp2; @@ -292,7 +292,7 @@ export function and(exp1, exp2): Expression { return res; } -export function or(exp1, exp2): Expression { +export function or(exp1: Expression, exp2: Expression): Expression { if (!isArray(exp1)) return exp1 ? exp1 : exp2; if (!isArray(exp2)) return exp2 ? exp2 : exp1; diff --git a/lib/ui/api.ts b/lib/ui/api.ts index e79d86e..3afd16c 100644 --- a/lib/ui/api.ts +++ b/lib/ui/api.ts @@ -28,6 +28,7 @@ import { getConfig } from "../local-cache"; import { QueryOptions, Expression } from "../types"; import { generateSalt, hashPassword } from "../auth"; import { del } from "../cache"; +import Authorizer from "../common/authorizer"; const router = new Router(); export default router; @@ -67,15 +68,20 @@ const resources = { }; router.get(`/devices/:id.csv`, async ctx => { + const authorizer: Authorizer = ctx.state.authorizer; const log = { message: "Query device (CSV)", context: ctx, id: ctx.params.id }; - const filter = ["=", ["PARAM", RESOURCE_IDS.devices], ctx.params.id]; + const filter = and(authorizer.getFilter("devices", 2), [ + "=", + ["PARAM", RESOURCE_IDS.devices], + ctx.params.id + ]); - if (!ctx.state.authorizer.hasAccess("devices", 2)) { + if (!authorizer.hasAccess("devices", 2)) { logUnauthorizedWarning(log); return void (ctx.status = 404); } @@ -115,8 +121,10 @@ router.get(`/devices/:id.csv`, async ctx => { for (const [resource, flags] of Object.entries(resources)) { router.head(`/${resource}`, async (ctx, next) => { - let filter: Expression = true; - if (ctx.request.query.filter) filter = parse(ctx.request.query.filter); + const authorizer: Authorizer = ctx.state.authorizer; + let filter: Expression = authorizer.getFilter(resource, 1); + if (ctx.request.query.filter) + filter = and(filter, parse(ctx.request.query.filter)); const log = { message: `Count ${resource}`, @@ -125,7 +133,7 @@ for (const [resource, flags] of Object.entries(resources)) { count: null }; - if (!ctx.state.authorizer.hasAccess(resource, 1)) { + if (!authorizer.hasAccess(resource, 1)) { logUnauthorizedWarning(log); return void next(); } @@ -148,9 +156,11 @@ for (const [resource, flags] of Object.entries(resources)) { }); router.get(`/${resource}`, async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const options: QueryOptions = {}; - let filter: Expression = true; - if (ctx.request.query.filter) filter = parse(ctx.request.query.filter); + let filter: Expression = authorizer.getFilter(resource, 2); + if (ctx.request.query.filter) + filter = and(filter, parse(ctx.request.query.filter)); if (ctx.request.query.limit) options.limit = +ctx.request.query.limit; if (ctx.request.query.skip) options.skip = +ctx.request.query.skip; if (ctx.request.query.sort) @@ -171,7 +181,7 @@ for (const [resource, flags] of Object.entries(resources)) { projection: options.projection }; - if (!ctx.state.authorizer.hasAccess(resource, 2)) { + if (!authorizer.hasAccess(resource, 2)) { logUnauthorizedWarning(log); return void next(); } @@ -199,9 +209,11 @@ for (const [resource, flags] of Object.entries(resources)) { // CSV download router.get(`/${resource}.csv`, async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const options: QueryOptions = { projection: {} }; - let filter: Expression = true; - if (ctx.request.query.filter) filter = parse(ctx.request.query.filter); + let filter: Expression = authorizer.getFilter(resource, 2); + if (ctx.request.query.filter) + filter = and(filter, parse(ctx.request.query.filter)); if (ctx.request.query.limit) options.limit = +ctx.request.query.limit; if (ctx.request.query.skip) options.skip = +ctx.request.query.skip; if (ctx.request.query.sort) @@ -216,7 +228,7 @@ for (const [resource, flags] of Object.entries(resources)) { sort: options.sort }; - if (!ctx.state.authorizer.hasAccess(resource, 2)) { + if (!authorizer.hasAccess(resource, 2)) { logUnauthorizedWarning(log); return void next(); } @@ -285,14 +297,19 @@ for (const [resource, flags] of Object.entries(resources)) { }); router.head(`/${resource}/:id`, async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const log = { message: `Count ${resource}`, context: ctx, filter: `${RESOURCE_IDS[resource]} = "${ctx.params.id}"` }; - const filter = ["=", ["PARAM", RESOURCE_IDS[resource]], ctx.params.id]; - if (!ctx.state.authorizer.hasAccess(resource, 2)) { + const filter = and(authorizer.getFilter(resource, 2), [ + "=", + ["PARAM", RESOURCE_IDS[resource]], + ctx.params.id + ]); + if (!authorizer.hasAccess(resource, 2)) { logUnauthorizedWarning(log); return void next(); } @@ -306,14 +323,19 @@ for (const [resource, flags] of Object.entries(resources)) { }); router.get(`/${resource}/:id`, async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const log = { message: `Query ${resource}`, context: ctx, filter: `${RESOURCE_IDS[resource]} = "${ctx.params.id}"` }; - const filter = ["=", ["PARAM", RESOURCE_IDS[resource]], ctx.params.id]; - if (!ctx.state.authorizer.hasAccess(resource, 2)) { + const filter = and(authorizer.getFilter(resource, 2), [ + "=", + ["PARAM", RESOURCE_IDS[resource]], + ctx.params.id + ]); + if (!authorizer.hasAccess(resource, 2)) { logUnauthorizedWarning(log); return void next(); } @@ -326,17 +348,20 @@ for (const [resource, flags] of Object.entries(resources)) { ctx.body = res[0]; }); - // TODO add PUT, PATCH routes if (flags & RESOURCE_DELETE) { router.delete(`/${resource}/:id`, async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const log = { message: `Delete ${resource}`, context: ctx, id: ctx.params.id }; - const authorizer = ctx.state.authorizer; - const filter = ["=", ["PARAM", RESOURCE_IDS[resource]], ctx.params.id]; + const filter = and(authorizer.getFilter(resource, 3), [ + "=", + ["PARAM", RESOURCE_IDS[resource]], + ctx.params.id + ]); if (!authorizer.hasAccess(resource, 3)) { logUnauthorizedWarning(log); return void next(); @@ -360,6 +385,7 @@ for (const [resource, flags] of Object.entries(resources)) { if (flags & RESOURCE_PUT) { router.put(`/${resource}/:id`, async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const id = ctx.params.id; const log = { @@ -368,7 +394,6 @@ for (const [resource, flags] of Object.entries(resources)) { id: id }; - const authorizer = ctx.state.authorizer; if (!authorizer.hasAccess(resource, 3)) { logUnauthorizedWarning(log); return void next(); @@ -399,6 +424,7 @@ for (const [resource, flags] of Object.entries(resources)) { } router.put("/files/:id", async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const resource = "files"; const id = ctx.params.id; @@ -409,7 +435,6 @@ router.put("/files/:id", async (ctx, next) => { metadata: null }; - const authorizer = ctx.state.authorizer; if (!authorizer.hasAccess(resource, 3)) { logUnauthorizedWarning(log); return void next(); @@ -436,6 +461,7 @@ router.put("/files/:id", async (ctx, next) => { }); router.post("/devices/:id/tasks", async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const log = { message: "Commit tasks", context: ctx, @@ -443,8 +469,11 @@ router.post("/devices/:id/tasks", async (ctx, next) => { tasks: null }; - const authorizer = ctx.state.authorizer; - const filter = ["=", ["PARAM", "DeviceID.ID"], ctx.params.id]; + const filter = and(authorizer.getFilter("devices", 3), [ + "=", + ["PARAM", "DeviceID.ID"], + ctx.params.id + ]); if (!authorizer.hasAccess("devices", 3)) { logUnauthorizedWarning(log); return void next(); @@ -481,6 +510,7 @@ router.post("/devices/:id/tasks", async (ctx, next) => { }); router.post("/devices/:id/tags", async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const log = { message: "Update tags", context: ctx, @@ -488,8 +518,11 @@ router.post("/devices/:id/tags", async (ctx, next) => { tags: ctx.request.body }; - const authorizer = ctx.state.authorizer; - const filter = ["=", ["PARAM", "DeviceID.ID"], ctx.params.id]; + const filter = and(authorizer.getFilter("devices", 3), [ + "=", + ["PARAM", "DeviceID.ID"], + ctx.params.id + ]); if (!authorizer.hasAccess("devices", 3)) { logUnauthorizedWarning(log); return void next(); @@ -528,6 +561,7 @@ router.get("/ping/:host", async ctx => { }); router.put("/users/:id/password", async (ctx, next) => { + const authorizer: Authorizer = ctx.state.authorizer; const username = ctx.params.id; const log = { message: "Change password", @@ -535,8 +569,6 @@ router.put("/users/:id/password", async (ctx, next) => { username: username }; - const authorizer = ctx.state.authorizer; - if (!ctx.state.user) { // User not logged in if ( @@ -556,7 +588,11 @@ router.put("/users/:id/password", async (ctx, next) => { return void next(); } - const filter = ["=", ["PARAM", RESOURCE_IDS.users], username]; + const filter = and(authorizer.getFilter("users", 3), [ + "=", + ["PARAM", RESOURCE_IDS.users], + username + ]); const res = await db.query("users", filter); if (!res.length) return void next();