From ca52b1bd44ed43076e4294349fd81555b52e9ce9 Mon Sep 17 00:00:00 2001 From: Reid Blomquist Date: Mon, 8 Jan 2024 14:27:01 -0800 Subject: [PATCH] [IAM-1155] - provide thrown error details in return from computePermissions so that clients ingesting might be better able to respond --- features/advanced.feature | 33 ++++++++++++++++++++++++++++++++ features/analyze.feature | 2 +- package-lock.json | 4 ++-- package.json | 2 +- src/examples/advanced/app.ts | 13 +++++++++++-- src/examples/advanced/can.ts | 21 +++++++++++++++----- src/permissions/compute.ts | 37 +++++++++++++++++++++++++----------- 7 files changed, 90 insertions(+), 22 deletions(-) diff --git a/features/advanced.feature b/features/advanced.feature index 8f7df43..b9cacf2 100644 --- a/features/advanced.feature +++ b/features/advanced.feature @@ -207,3 +207,36 @@ Scenario: deprecate.rewrite get todos And the response headers at deprecated-for is "GET /todos/${U.username}" And the response headers at location contains "/todos/${U.username}" +# Compute permissions + +Scenario: get positive TODO permissions + Given a user U with { "username": "${random}", "name": "Ringo ${random}" } + And a todo T with { "owner": "${U.username}", "item": "Autograph a photo for Marge" } + When GET /todo-permissions/${T.id}/${U.username} + Then the response is 200 and the payload includes + """ + { + "view": true, + "admin": true + } + """ + +Scenario: get negative TODO permissions with errors + Given a user U with { "username": "${random}-1", "name": "Ringo ${random}" } + And a user O with { "username": "${random}-2", "name": "John ${random}" } + And a todo T with { "owner": "${O.username}", "item": "Autograph a photo for Marge" } + When GET /todo-permissions/${T.id}/${U.username} + Then the response is 200 and the payload includes + """ + { + "view": false, + "admin": false, + "$errors": { + "admin": { + "statusCode": 401, + "error": "NOT_AUTHORIZED", + "extra": {} + } + } + } + """ \ No newline at end of file diff --git a/features/analyze.feature b/features/analyze.feature index eaeff2a..df340b4 100644 --- a/features/analyze.feature +++ b/features/analyze.feature @@ -35,4 +35,4 @@ Scenario: analyze basic app Scenario: analyze advanced app succeeds Given an advanced app When analyzing the current app - Then the analyzed routes at length is 17 \ No newline at end of file + Then the analyzed routes at length is 18 \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index abaa99c..aeb7913 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "async-app", - "version": "4.8.1", + "version": "4.9.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "async-app", - "version": "4.8.1", + "version": "4.9.0", "license": "MIT", "dependencies": { "express": "^4.16.3", diff --git a/package.json b/package.json index fa06965..28a3c0c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "async-app", - "version": "4.8.1", + "version": "4.9.0", "description": "An express wrapper for handling async middlewares, order middlewares, schema validator, and other stuff", "type": "commonjs", "main": "dist/index.js", diff --git a/src/examples/advanced/app.ts b/src/examples/advanced/app.ts index df10d6f..5d9e0ab 100644 --- a/src/examples/advanced/app.ts +++ b/src/examples/advanced/app.ts @@ -7,9 +7,9 @@ import bodyParser from 'body-parser'; import express from 'express'; import { join } from 'path'; -import { createCustomResponse, deprecate } from '../..'; +import { computePermissions, createCustomResponse, deprecate } from '../..'; import createApp, { Req } from './async-app'; -import can from './can'; +import can, { entities } from './can'; import { addTodo, addUser, getTodosForUser } from './db'; import load from './load'; import purgeUser from './purge-user'; @@ -125,6 +125,15 @@ app.get( (req: Req<'todo'>) => req.todo, ); +app.get( + '/todo-permissions/:todoId/:username', + 'Returns computed permissions for the specified TODO and user', + load.todo.fromParams(), + load.user.fromParams(), + (req: Req<'todo' | 'user'>) => + computePermissions(entities, 'todo', { todo: req.todo, user: req.user }), +); + app.get('/echo1', () => 'echo'); app.get( diff --git a/src/examples/advanced/can.ts b/src/examples/advanced/can.ts index 696658b..3e94d6c 100644 --- a/src/examples/advanced/can.ts +++ b/src/examples/advanced/can.ts @@ -3,13 +3,24 @@ // +========================================================================+ // // In your case this is `from 'async-app'` -import { createPermissions } from '../..'; +import { createPermissions, CustomError } from '../..'; import { ExampleEntities } from './async-app'; -const can = createPermissions({ - todo: { - view: ({ user, todo }) => todo.owner === user.username, +const todo = { + admin: ({ user, todo }: Pick) => { + if (todo.owner === user.username) { + return true; + } + throw new CustomError(401, 'NOT_AUTHORIZED'); }, -}); + view: ({ user, todo }: Pick) => + todo.owner === user.username, +}; + +export const entities = { + todo, +}; + +const can = createPermissions(entities); export default can; diff --git a/src/permissions/compute.ts b/src/permissions/compute.ts index 105a871..d454a6d 100644 --- a/src/permissions/compute.ts +++ b/src/permissions/compute.ts @@ -9,9 +9,11 @@ import { } from './types'; import { getKeys } from './util'; -interface Permissions { - [name: string]: boolean; -} +type Permissions = { + [key in T]: key extends '$errors' ? Record : boolean; +} & { + $errors: Record; +}; const areEqual = (arr1: T[], arr2: T[]) => arr1.length === arr2.length && arr1.every(item => arr2.includes(item)); @@ -54,9 +56,9 @@ const tryPermission = ( requiredModels: TEntities, ) => { try { - return permissionFn(requiredModels); - } catch (_) { - return false; + return { access: permissionFn(requiredModels) }; + } catch (error) { + return { access: false, error }; } }; @@ -68,6 +70,8 @@ const assertEntity = ( return entity; }; +// Accepting an optional tryFunction lets users override the default try/catch +// which means you can in theory pull off the `extra` key in thrown errors etc export const computePermissions = ( entities: PermissionMap, entityName: keyof TEntities, @@ -77,15 +81,21 @@ export const computePermissions = ( checkExpectedModels(entity, entityName, requiredModels); - const permissions = {} as Permissions; + const permissions = { + $errors: {}, + } as Permissions; Object.keys(entity).forEach((action) => { const spec = entity[action]; // load permission on entire entity, e.g.: $permissions.delete = true if (isPermissionFn(spec)) { - const permission = tryPermission(spec, requiredModels); - permissions[action] = permission; + const { access, error } = tryPermission(spec, requiredModels); + permissions[action] = access; + + if (error) { + (permissions.$errors)[action] = error; + } } // load subpermissions, e.g.: $permissions['delete.editHash'] = true @@ -93,9 +103,14 @@ export const computePermissions = ( Object.keys(spec).forEach((subaction) => { const permissionFn = spec[subaction]; if (isPermissionFn(permissionFn)) { - const permission = tryPermission(permissionFn, requiredModels); + const { access, error } = tryPermission(permissionFn, requiredModels); + const permKey = `${action}.${subaction}`; + + permissions[permKey] = access; - permissions[`${action}.${subaction}`] = permission; + if (error) { + (permissions.$errors)[permKey] = error; + } } }); }