From f280052d93cc1e867d227e9cdcce7fa6066a2a71 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Fri, 18 Oct 2024 15:33:47 +0200 Subject: [PATCH 1/2] refactor: Permission model sanitize method --- packages/backend/src/models/permission.js | 36 ++++++++++++++++------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/backend/src/models/permission.js b/packages/backend/src/models/permission.js index 6ef9ae0d..a58aa53b 100644 --- a/packages/backend/src/models/permission.js +++ b/packages/backend/src/models/permission.js @@ -19,25 +19,39 @@ class Permission extends Base { }, }; - static sanitize(permissions) { + static filter(permissions) { const sanitizedPermissions = permissions.filter((permission) => { const { action, subject, conditions } = permission; - const relevantAction = permissionCatalog.actions.find( - (actionCatalogItem) => actionCatalogItem.key === action - ); - const validSubject = relevantAction.subjects.includes(subject); - const validConditions = conditions.every((condition) => { - return !!permissionCatalog.conditions.find( - (conditionCatalogItem) => conditionCatalogItem.key === condition - ); - }); + const relevantAction = this.findAction(action); + const validSubject = this.isSubjectValid(subject, relevantAction); + const validConditions = this.areConditionsValid(conditions); - return validSubject && validConditions; + return relevantAction && validSubject && validConditions; }); return sanitizedPermissions; } + + static findAction(action) { + return permissionCatalog.actions.find( + (actionCatalogItem) => actionCatalogItem.key === action + ); + } + + static isSubjectValid(subject, action) { + return action && action.subjects.includes(subject); + } + + static areConditionsValid(conditions) { + return conditions.every((condition) => this.isConditionValid(condition)); + } + + static isConditionValid(condition) { + return !!permissionCatalog.conditions.find( + (conditionCatalogItem) => conditionCatalogItem.key === condition + ); + } } export default Permission; From bc0861fd9e1417f87dbc07e76ca9cd6ef2d47f99 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Fri, 18 Oct 2024 15:34:06 +0200 Subject: [PATCH 2/2] test: Implement tests for permission model --- .../__snapshots__/permission.test.js.snap | 42 ++++++++ .../backend/src/models/permission.test.js | 95 +++++++++++++++++++ packages/backend/src/models/role.js | 4 +- 3 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 packages/backend/src/models/__snapshots__/permission.test.js.snap create mode 100644 packages/backend/src/models/permission.test.js diff --git a/packages/backend/src/models/__snapshots__/permission.test.js.snap b/packages/backend/src/models/__snapshots__/permission.test.js.snap new file mode 100644 index 00000000..4b861e39 --- /dev/null +++ b/packages/backend/src/models/__snapshots__/permission.test.js.snap @@ -0,0 +1,42 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`Permission model > jsonSchema should have correct validations 1`] = ` +{ + "properties": { + "action": { + "minLength": 1, + "type": "string", + }, + "conditions": { + "items": { + "type": "string", + }, + "type": "array", + }, + "createdAt": { + "type": "string", + }, + "id": { + "format": "uuid", + "type": "string", + }, + "roleId": { + "format": "uuid", + "type": "string", + }, + "subject": { + "minLength": 1, + "type": "string", + }, + "updatedAt": { + "type": "string", + }, + }, + "required": [ + "roleId", + "action", + "subject", + ], + "type": "object", +} +`; diff --git a/packages/backend/src/models/permission.test.js b/packages/backend/src/models/permission.test.js new file mode 100644 index 00000000..c53b8218 --- /dev/null +++ b/packages/backend/src/models/permission.test.js @@ -0,0 +1,95 @@ +import { describe, it, expect } from 'vitest'; +import Permission from './permission'; +import permissionCatalog from '../helpers/permission-catalog.ee.js'; + +describe('Permission model', () => { + it('tableName should return correct name', () => { + expect(Permission.tableName).toBe('permissions'); + }); + + it('jsonSchema should have correct validations', () => { + expect(Permission.jsonSchema).toMatchSnapshot(); + }); + + it('filter should return only valid permissions based on permission catalog', () => { + const permissions = [ + { action: 'read', subject: 'Flow', conditions: ['isCreator'] }, + { action: 'delete', subject: 'Connection', conditions: [] }, + { action: 'publish', subject: 'Flow', conditions: ['isCreator'] }, + { action: 'update', subject: 'Execution', conditions: [] }, // Invalid subject + { action: 'read', subject: 'Execution', conditions: ['invalid'] }, // Invalid condition + { action: 'invalid', subject: 'Execution', conditions: [] }, // Invalid action + ]; + + const result = Permission.filter(permissions); + + expect(result).toStrictEqual([ + { action: 'read', subject: 'Flow', conditions: ['isCreator'] }, + { action: 'delete', subject: 'Connection', conditions: [] }, + { action: 'publish', subject: 'Flow', conditions: ['isCreator'] }, + ]); + }); + + describe('findAction', () => { + it('should return action from permission catalog', () => { + const action = Permission.findAction('create'); + expect(action.key).toStrictEqual('create'); + }); + + it('should return undefined for invalid actions', () => { + const invalidAction = Permission.findAction('invalidAction'); + expect(invalidAction).toBeUndefined(); + }); + }); + + describe('isSubjectValid', () => { + it('should return true for valid subjects', () => { + const validAction = permissionCatalog.actions.find( + (action) => action.key === 'create' + ); + + const validSubject = Permission.isSubjectValid('Connection', validAction); + expect(validSubject).toBe(true); + }); + + it('should return false for invalid subjects', () => { + const validAction = permissionCatalog.actions.find( + (action) => action.key === 'create' + ); + + const invalidSubject = Permission.isSubjectValid( + 'Execution', + validAction + ); + + expect(invalidSubject).toBe(false); + }); + }); + + describe('areConditionsValid', () => { + it('should return true for valid conditions', () => { + const validConditions = Permission.areConditionsValid(['isCreator']); + expect(validConditions).toBe(true); + }); + + it('should return false for invalid conditions', () => { + const invalidConditions = Permission.areConditionsValid([ + 'invalidCondition', + ]); + + expect(invalidConditions).toBe(false); + }); + }); + + describe('isConditionValid', () => { + it('should return true for valid conditions', () => { + const validCondition = Permission.isConditionValid('isCreator'); + expect(validCondition).toBe(true); + }); + + it('should return false for invalid conditions', () => { + const invalidCondition = Permission.isConditionValid('invalidCondition'); + expect(invalidCondition).toBe(false); + }); + }); +}); diff --git a/packages/backend/src/models/role.js b/packages/backend/src/models/role.js index 6760c321..dec80c5b 100644 --- a/packages/backend/src/models/role.js +++ b/packages/backend/src/models/role.js @@ -63,14 +63,14 @@ class Role extends Base { await this.$relatedQuery('permissions', trx).delete(); if (permissions?.length) { - const sanitizedPermissions = Permission.sanitize(permissions).map( + const validPermissions = Permission.filter(permissions).map( (permission) => ({ ...permission, roleId: this.id, }) ); - await Permission.query().insert(sanitizedPermissions); + await Permission.query().insert(validPermissions); } await this.$query(trx).patch({