From 593e5f8714f142b7df760a138275259e80960cc1 Mon Sep 17 00:00:00 2001 From: Mohamed El-Bably Date: Sun, 16 Oct 2022 14:39:49 +0200 Subject: [PATCH] fix(attributes): merge attributes when roles overlapped docs(attributes): update attributes merge examples --- README.md | 45 +++++---- package.json | 2 +- src/simpleAccess.ts | 122 +++++++++++++------------ test/data/roles.ts | 2 +- test/simpleAccess/simpleAccess.spec.ts | 16 ++-- 5 files changed, 99 insertions(+), 88 deletions(-) diff --git a/README.md b/README.md index 862ccb4..29a9c5c 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ Attribute-Role-Based Hybrid Access Control Library [![Node Version](https://img.shields.io/node/v/simple-access.svg)](https://nodejs.org) [![npm version](https://img.shields.io/npm/v/simple-access/latest.svg)](https://www.npmjs.com/package/simple-access) [![Build Status](https://github.com/m-elbably/simple-access/workflows/simple-access/badge.svg)](https://github.com/m-elbably/simple-access/workflows/simple-access/badge.svg) -[![Coverage Status](https://coveralls.io/repos/github/m-elbably/simple-access/badge.svg?branch=main&t=pKbxsM)](https://coveralls.io/github/m-elbably/simple-access?branch=main) +[![Coverage Status](https://coveralls.io/repos/github/m-elbably/simple-access/badge.svg?branch=master&t=pKbxsM)](https://coveralls.io/github/m-elbably/simple-access?branch=master) [![License](https://img.shields.io/github/license/m-elbably/simple-access.svg)](https://raw.githubusercontent.com/m-elbably/simple-access/master/LICENSE) ## Installation @@ -59,20 +59,20 @@ Role is the level of access given to subject (user or business entity) when this **Role Schema** ```json { - "name": "string", - "resources": [ - { - "name": "string", - "actions": [ - { - "name": "string", - "attributes": ["string"], - "conditions": ["object"], - "scope": "object" - } - ] - } - ] + "name": "string", + "resources": [ + { + "name": "string", + "actions": [ + { + "name": "string", + "attributes": ["string"], + "conditions": ["object"], + "scope": "object" + } + ] + } + ] } ``` @@ -119,12 +119,10 @@ export class MemoryAdapter extends BaseAdapter { constructor(roles?: Array) { super("MemoryAdapter"); - if (roles != null) { - this.addRoles(roles); - } + this.addRoles(roles); } - addRoles(roles: Array): void { + setRoles(roles: Array): void { this._roles = roles; this._cache = {}; this._roles.forEach((role: Role) => { @@ -258,8 +256,9 @@ The returned permission - **Attributes** are merged, and the most permissive attributes will override. - Attributes `["*"]` will override attributes `["name", "age", "!address"]` - Attributes `["name", "age"]` will merge with attributes `["address"]` into new attributes array `["name", "age", "address"]` - - Attributes `["name", "age"]` will be merged with attributes `["image", "!address"]` into new attributes array `["name", "age", "image", "!address"]` - - Attributes `["*", "name"]` will be merged with attributes `["image", "!address"]` into new attributes array `["*", "!address"]` and `["name" , "image"]` will be omitted as the first role has access to `*` attributes + - Attributes `["*", "!address"]` will be merged with attributes `["age"]` into new attributes array `["*", "!address"]` + - Attributes `["*", "!age"]` will be merged with attributes `["*", "!image", "!address"]` into new attributes array `["*"]` and `["age" , "image", "address"]` will be allowed because they are not negated from both sides + - Attributes `["*", "!age"]` will be merged with attributes `["image"]` into new attributes array `["*", "!age"]` and `"image"` attributes will be omitted because its by default allowed >**Projected** (Allowed) attributes gets merged based on a **union** operation
>**Negated** (Not allowed) attributes gets merged based on **intersection** operation @@ -310,7 +309,7 @@ You can use a set of MongoDB queries in JavaScript, like following operators: You can do this check using `canSubjectAccessResource` function:
`canSubjectAccessResource(permission: Permission, subject: Object, resource: Object): boolean` -You can refer to specific attributes in subject with `subject.` and refer to attributes in resource `resource.` as a prefix, and it will substituted with the actual values before evaluation. +You can refer to specific attributes in subject with `subject.` and refer to attributes in resource `resource.` as a prefix, and it will be substituted with the actual values before evaluation. **Example:** ```json @@ -359,7 +358,7 @@ if(permission.granted) { >Simple Access is depending on [Floppy Filter](https://github.com/m-elbably/floppy-filter) library for complex object filtering, Please check its documentation to know how to select or negate attributes in an efficient way. -You can do this check using `filter` function:
+You can do this using `filter` function:
`filter(permission: Permission, data: Object): Object` **Example:** diff --git a/package.json b/package.json index b33a22c..cbac080 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "simple-access", - "version": "1.0.1", + "version": "1.0.2", "description": "Attribute-Role-Based Hybrid Access Control Library", "homepage": "https://github.com/m-elbably/simple-access#readme", "repository": "https://github.com/m-elbably/simple-access", diff --git a/src/simpleAccess.ts b/src/simpleAccess.ts index 2b51848..6642860 100644 --- a/src/simpleAccess.ts +++ b/src/simpleAccess.ts @@ -1,8 +1,8 @@ -import * as _ from 'lodash'; +import * as _ from "lodash"; import Ajv from "ajv"; import { Tuple, Role, ErrorEx, Action } from "./types"; import { BaseAdapter } from "./adapters"; -import { Utils } from "./core/utils"; +import { Utils } from "./core"; import { PermissionOptions, Permission } from "./types"; import { roleSchema } from "./validation"; @@ -73,7 +73,7 @@ export class SimpleAccess { } resource.actions.forEach((action) => { - const iAction: Action = { + const currentAction: Action = { name: action.name, attributes: action.attributes ? Array.from(action.attributes) @@ -90,74 +90,84 @@ export class SimpleAccess { return; } - cachedAction = resources[resource.name][iAction.name]; + cachedAction = resources[resource.name][currentAction.name]; if (cachedAction == null) { - resources[resource.name][iAction.name] = iAction; + resources[resource.name][currentAction.name] = + currentAction; } else { // Check and merge attributes - const actionAllAttrs = - iAction.attributes.length === 1 && - iAction.attributes[0] === ALL; - const rActionAllAttrs = + const currentActionAllowAllEx = + currentAction.attributes.length === 1 && + currentAction.attributes[0] === ALL; + const cachedActionAllowAllEx = cachedAction.attributes.length === 1 && cachedAction.attributes[0] === ALL; - if (!rActionAllAttrs) { - if (actionAllAttrs) { + if (!cachedActionAllowAllEx) { + if (currentActionAllowAllEx) { // If action or cached actions = ['*'] then, no need to merge // We take the most permissive attributes cachedAction.attributes = [ALL]; } else { - let startsWithAll = false; - const rAttributes = []; - - if (iAction.attributes[0] === ALL) { - startsWithAll = true; - iAction.attributes.splice(0, 1); - } else if (cachedAction.attributes[0] === ALL) { - startsWithAll = true; - cachedAction.attributes.splice(0, 1); + const attributesBuffer = []; + const currentActionAllowAll = + currentAction.attributes[0] === ALL; + const cachedActionAllowAll = + cachedAction.attributes[0] === ALL; + + if ( + !currentActionAllowAll && + !cachedActionAllowAll + ) { + const projected = _.filter( + _.union( + currentAction.attributes, + cachedAction.attributes + ), + (a) => a != null && !a.startsWith("!") + ); + + attributesBuffer.push(...projected); + } else if ( + currentActionAllowAll && + cachedActionAllowAll + ) { + const negated = _.filter( + _.intersection( + currentAction.attributes, + cachedAction.attributes + ), + (a) => a != null && a.startsWith("!") + ); + + attributesBuffer.push(ALL, ...negated); + } else if ( + currentActionAllowAll || + cachedActionAllowAll + ) { + const negated = _.filter( + _.union( + currentAction.attributes, + cachedAction.attributes + ), + (a) => a != null && a.startsWith("!") + ); + + attributesBuffer.push(ALL, ...negated); } - // Select projection attributes with union - const projected = _.filter( - _.union( - iAction.attributes, - cachedAction.attributes - ), - (a) => a != null && !a.startsWith("!") - ); - - // Select negated attributes with intersection - // Different negated attributes in both side means it's allowed on both due to its absence - const negated = _.filter( - _.intersection( - iAction.attributes, - cachedAction.attributes - ), - (a) => a != null && a.startsWith("!") - ); - - // Add the union between selected and negated attributes, based on wild card operator '*' - if (startsWithAll) { - rAttributes.push(ALL); - rAttributes.push(...negated); - } else { - rAttributes.push(...projected); - } - - cachedAction.attributes = rAttributes; + cachedAction.attributes = attributesBuffer; } } // Check conditions - if (iAction.conditions == null) { - iAction.conditions = []; + if (currentAction.conditions == null) { + currentAction.conditions = []; } const isEmptyConditions = - iAction.conditions.length === 0; + currentAction.conditions.length === 0; const isEmptyCachedConditions = cachedAction.conditions.length === 0; @@ -168,18 +178,18 @@ export class SimpleAccess { } else { if (!isEmptyCachedConditions) { cachedAction.conditions.push( - ...iAction.conditions + ...currentAction.conditions ); } } // Check scope - if (iAction.scope == null) { - iAction.scope = {}; + if (currentAction.scope == null) { + currentAction.scope = {}; } const isScopeEmpty = - Object.entries(iAction.scope).length === 0; + Object.entries(currentAction.scope).length === 0; const isCachedScopeEmpty = Object.entries(cachedAction.scope).length === 0; if (isScopeEmpty) { @@ -188,7 +198,7 @@ export class SimpleAccess { } } else { if (!isCachedScopeEmpty) { - Object.entries(iAction.scope).forEach( + Object.entries(currentAction.scope).forEach( ([k, v]) => { cachedAction.scope[k] = v; } diff --git a/test/data/roles.ts b/test/data/roles.ts index d0942d4..eacbe6c 100644 --- a/test/data/roles.ts +++ b/test/data/roles.ts @@ -30,7 +30,7 @@ export const Roles = [ name: RESOURCES.PRODUCT, actions: [ { name: "create", attributes: ["*", "active"] }, - { name: "read", attributes: ["*", "active"] }, + { name: "read", attributes: ["active"] }, { name: "update", attributes: ["*", "!history"] }, { name: "delete", attributes: ["*"] }, ], diff --git a/test/simpleAccess/simpleAccess.spec.ts b/test/simpleAccess/simpleAccess.spec.ts index c4439bd..a08673b 100644 --- a/test/simpleAccess/simpleAccess.spec.ts +++ b/test/simpleAccess/simpleAccess.spec.ts @@ -16,7 +16,7 @@ before(() => { describe("Test core functionalities", () => { it("Should return validation error, for invalid adapter", async () => { try { - const simpleAccess = new SimpleAccess(undefined); + new SimpleAccess(undefined); } catch (e) { expect(e) .to.be.instanceOf(Error) @@ -320,6 +320,7 @@ describe("Test can functionality with overlapped roles - attributes", () => { it("Should return permission object with all allowed attributes in action - all attributes", async () => { const ACTION_NAME = "read"; const RESOURCE_NAME = RESOURCES.PRODUCT; + const RESULT = ["*", "!history"]; const permission = await acl.can( [ROLES.ADMINISTRATOR, ROLES.OPERATION], ACTION_NAME, @@ -334,13 +335,13 @@ describe("Test can functionality with overlapped roles - attributes", () => { .with.ownProperty(ACTION_NAME) .with.ownProperty("attributes") .to.be.an("array") - .eql(["*"]); + .eql(RESULT); expect(permission) .to.be.an("object") .with.property("attributes") .to.be.an("array") - .eql(["*"]); + .eql(RESULT); }); it("Should return permission object with all allowed attributes in action - filtered all attributes", async () => { @@ -399,6 +400,7 @@ describe("Test can functionality with overlapped roles - attributes", () => { it("Should return permission object with all allowed attributes in action - mixed attributes", async () => { const ACTION_NAME = "read"; const RESOURCE_NAME = RESOURCES.PRODUCT; + const RESULT = ["*", "!history"]; const permission = await acl.can( [ROLES.ADMINISTRATOR, ROLES.OPERATION], ACTION_NAME, @@ -413,13 +415,13 @@ describe("Test can functionality with overlapped roles - attributes", () => { .with.ownProperty(ACTION_NAME) .with.ownProperty("attributes") .to.be.an("array") - .eql(["*"]); + .eql(RESULT); expect(permission) .to.be.an("object") .with.property("attributes") .to.be.an("array") - .eql(["*"]); + .eql(RESULT); }); it("Should return permission object with all allowed attributes in action - negated attributes", async () => { @@ -788,7 +790,7 @@ describe("Test data filtration base on permission", () => { const resource = acl.filter(permission, PRODUCTS[0]); expect(resource).to.be.an("object").to.have.ownProperty("authorId"); - expect(resource).to.be.an("object").to.not.have.ownProperty("isAction"); + expect(resource).to.be.an("object").to.not.have.ownProperty("isActive"); }); }); @@ -816,6 +818,6 @@ describe("Test permission bounded functionalities", () => { const resource = permission.filter(PRODUCTS[0]); expect(resource).to.be.an("object").to.have.ownProperty("authorId"); - expect(resource).to.be.an("object").to.not.have.ownProperty("isAction"); + expect(resource).to.be.an("object").to.not.have.ownProperty("isActive"); }); });