From d24f04912e70b826097335f4baae5ef832855b79 Mon Sep 17 00:00:00 2001 From: Joan Sanchez Date: Tue, 26 May 2020 14:37:34 +0200 Subject: [PATCH 1/4] 2205 - Prevent relations to being logged in --- db/changes/10180-holyWeek/00-ACL.sql | 1 + loopback/common/models/loggable.js | 129 +++++++++++++++++-------- modules/worker/front/log/locale/es.yml | 1 + modules/worker/front/routes.json | 3 +- 4 files changed, 92 insertions(+), 42 deletions(-) create mode 100644 db/changes/10180-holyWeek/00-ACL.sql diff --git a/db/changes/10180-holyWeek/00-ACL.sql b/db/changes/10180-holyWeek/00-ACL.sql new file mode 100644 index 000000000..d4b961e22 --- /dev/null +++ b/db/changes/10180-holyWeek/00-ACL.sql @@ -0,0 +1 @@ +INSERT INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) VALUES ('WorkerLog', '*', 'READ', 'ALLOW', 'ROLE', 'hr'); \ No newline at end of file diff --git a/loopback/common/models/loggable.js b/loopback/common/models/loggable.js index d9116a0de..61a571607 100644 --- a/loopback/common/models/loggable.js +++ b/loopback/common/models/loggable.js @@ -2,6 +2,7 @@ const pick = require('object.pick'); const LoopBackContext = require('loopback-context'); module.exports = function(Self) { + // const relations = ctx.Model.relations; Self.setup = function() { Self.super_.setup.call(this); }; @@ -12,23 +13,33 @@ module.exports = function(Self) { }); Self.observe('before save', async function(ctx) { - let options = {}; + const appModels = ctx.Model.app.models; + const options = {}; + + // Check for transactions if (ctx.options && ctx.options.transaction) options.transaction = ctx.options.transaction; let oldInstance; - let oldInstanceFk; let newInstance; if (ctx.data) { - oldInstanceFk = pick(ctx.currentInstance, Object.keys(ctx.data)); + const changes = pick(ctx.currentInstance, Object.keys(ctx.data)); newInstance = await fkToValue(ctx.data, ctx); - oldInstance = await fkToValue(oldInstanceFk, ctx); + oldInstance = await fkToValue(changes, ctx); + if (ctx.where && !ctx.currentInstance) { - let fields = Object.keys(ctx.data); - ctx.oldInstances = await ctx.Model.app.models[ctx.Model.definition.name].find({where: ctx.where, fields: fields}, options); + const fields = Object.keys(ctx.data); + const modelName = modelDef.name; + + ctx.oldInstances = await appModels[modelName].find({ + where: ctx.where, + fields: fields + }, options); } } + + // Get changes from created instance if (ctx.isNewInstance) newInstance = await fkToValue(ctx.instance.__data, ctx); @@ -37,18 +48,24 @@ module.exports = function(Self) { }); Self.observe('before delete', async function(ctx) { + const appModels = ctx.Model.app.models; + const definition = ctx.Model.definition; + const relations = ctx.Model.relations; + let options = {}; if (ctx.options && ctx.options.transaction) options.transaction = ctx.options.transaction; if (ctx.where) { - let affectedModel = ctx.Model.definition.name; - let definition = ctx.Model.definition; - let deletedInstances = await ctx.Model.app.models[affectedModel].find({where: ctx.where}, options); + let affectedModel = definition.name; + let deletedInstances = await appModels[affectedModel].find({ + where: ctx.where + }, options); + let relation = definition.settings.log.relation; if (relation) { - let primaryKey = ctx.Model.relations[relation].keyFrom; + let primaryKey = relations[relation].keyFrom; let arrangedDeletedInstances = []; for (let i = 0; i < deletedInstances.length; i++) { @@ -69,6 +86,8 @@ module.exports = function(Self) { }); async function logDeletedInstances(ctx, loopBackContext) { + const appModels = ctx.Model.app.models; + const modelDef = ctx.Model.definition; let options = {}; if (ctx.options && ctx.options.transaction) options.transaction = ctx.options.transaction; @@ -78,14 +97,14 @@ module.exports = function(Self) { if (loopBackContext) userFk = loopBackContext.active.accessToken.userId; - let definition = ctx.Model.definition; + let definition = modelDef; let changedModelValue = definition.settings.log.changedModelValue; let logRecord = { originFk: instance.originFk, userFk: userFk, action: 'delete', - changedModel: ctx.Model.definition.name, + changedModel: modelDef.name, changedModelId: instance.id, changedModelValue: instance[changedModelValue], oldInstance: instance, @@ -95,26 +114,44 @@ module.exports = function(Self) { delete instance.originFk; let logModel = definition.settings.log.model; - await ctx.Model.app.models[logModel].create(logRecord, options); + await appModels[logModel].create(logRecord, options); }); } + // Get log values from a foreign key async function fkToValue(instance, ctx) { + const appModels = ctx.Model.app.models; + const relations = ctx.Model.relations; let options = {}; + + // Check for transactions if (ctx.options && ctx.options.transaction) options.transaction = ctx.options.transaction; - let cleanInstance = JSON.parse(JSON.stringify(instance)); - let result = {}; - for (let key in cleanInstance) { - let val = cleanInstance[key]; - if (val === undefined || val === null) continue; - for (let key1 in ctx.Model.relations) { - let val1 = ctx.Model.relations[key1]; - if (val1.keyFrom == key && key != 'id') { - let recordSet = await ctx.Model.app.models[val1.modelTo.modelName].findById(val, null, options); + const instanceCopy = JSON.parse(JSON.stringify(instance)); + const result = {}; + for (const key in instanceCopy) { + let value = instanceCopy[key]; + + if (value instanceof Object) + continue; + + if (value === undefined || value === null) continue; + + for (let relationName in relations) { + const relation = relations[relationName]; + if (relation.keyFrom == key && key != 'id') { + const model = relation.modelTo; + const modelName = relation.modelTo.modelName; + const properties = model && model.definition.properties; + const settings = model && model.definition.settings; + + const recordSet = await appModels[modelName].findById(value, null, options); + + const hasShowField = settings.log && settings.log.showField; + let showField = hasShowField && recordSet + && recordSet[settings.log.showField]; - let showField = val1.modelTo && val1.modelTo.definition.settings.log && val1.modelTo.definition.settings.log.showField && recordSet && recordSet[val1.modelTo.definition.settings.log.showField]; if (!showField) { const showFieldNames = [ 'name', @@ -122,7 +159,10 @@ module.exports = function(Self) { 'code' ]; for (field of showFieldNames) { - if (val1.modelTo.definition.properties && val1.modelTo.definition.properties[field] && recordSet && recordSet[field]) { + const propField = properties && properties[field]; + const recordField = recordSet && recordSet[field]; + + if (propField && recordField) { showField = field; break; } @@ -130,25 +170,29 @@ module.exports = function(Self) { } if (showField && recordSet && recordSet[showField]) { - val = recordSet[showField]; + value = recordSet[showField]; break; } - val = recordSet && recordSet.id || val; + value = recordSet && recordSet.id || value; break; } } - result[key] = val; + result[key] = value; } return result; } async function logInModel(ctx, loopBackContext) { - let options = {}; + const appModels = ctx.Model.app.models; + const definition = ctx.Model.definition; + const defSettings = ctx.Model.definition.settings; + const relations = ctx.Model.relations; + + const options = {}; if (ctx.options && ctx.options.transaction) options.transaction = ctx.options.transaction; - let definition = ctx.Model.definition; let primaryKey; for (let property in definition.properties) { if (definition.properties[property].id) { @@ -163,11 +207,11 @@ module.exports = function(Self) { // RELATIONS LOG let changedModelId; - if (ctx.instance && !definition.settings.log.relation) { + if (ctx.instance && !defSettings.log.relation) { originId = ctx.instance.id; changedModelId = ctx.instance.id; - } else if (definition.settings.log.relation) { - primaryKey = ctx.Model.relations[definition.settings.log.relation].keyFrom; + } else if (defSettings.log.relation) { + primaryKey = relations[defSettings.log.relation].keyFrom; if (ctx.where && ctx.where[primaryKey]) originId = ctx.where[primaryKey]; @@ -181,12 +225,16 @@ module.exports = function(Self) { } // Sets the changedModelValue to save and the instances changed in case its an updateAll - let showField = definition.settings.log.showField; + let showField = defSettings.log.showField; let where; if (showField && (!ctx.instance || !ctx.instance[showField]) && ctx.where) { changedModelId = []; where = []; - let changedInstances = await ctx.Model.app.models[definition.name].find({where: ctx.where, fields: ['id', showField, primaryKey]}, options); + let changedInstances = await appModels[definition.name].find({ + where: ctx.where, + fields: ['id', showField, primaryKey] + }, options); + changedInstances.forEach(element => { where.push(element[showField]); changedModelId.push(element.id); @@ -195,7 +243,6 @@ module.exports = function(Self) { } else if (ctx.hookState.oldInstance) where = ctx.instance[showField]; - // Set oldInstance, newInstance, userFk and action let oldInstance = {}; if (ctx.hookState.oldInstance) @@ -211,14 +258,14 @@ module.exports = function(Self) { let action = setActionType(ctx); - removeUnloggableProperties(definition, oldInstance); - removeUnloggableProperties(definition, newInstance); + removeUnloggable(definition, oldInstance); + removeUnloggable(definition, newInstance); let logRecord = { originFk: originId, userFk: userFk, action: action, - changedModel: ctx.Model.definition.name, + changedModel: definition.name, changedModelId: changedModelId, // Model property with an different data type will throw a NaN error changedModelValue: where, oldInstance: oldInstance, @@ -226,9 +273,9 @@ module.exports = function(Self) { }; let logsToSave = setLogsToSave(where, changedModelId, logRecord, ctx); - let logModel = definition.settings.log.model; + let logModel = defSettings.log.model; - await ctx.Model.app.models[logModel].create(logsToSave, options); + await appModels[logModel].create(logsToSave, options); } /** @@ -236,7 +283,7 @@ module.exports = function(Self) { * @param {*} definition Model definition * @param {*} properties Modified object properties */ - function removeUnloggableProperties(definition, properties) { + function removeUnloggable(definition, properties) { const propList = Object.keys(properties); const propDefs = new Map(); diff --git a/modules/worker/front/log/locale/es.yml b/modules/worker/front/log/locale/es.yml index d9c204e55..c48c571ac 100644 --- a/modules/worker/front/log/locale/es.yml +++ b/modules/worker/front/log/locale/es.yml @@ -1,3 +1,4 @@ +Date: Fecha Model: Modelo Action: Acción Author: Autor diff --git a/modules/worker/front/routes.json b/modules/worker/front/routes.json index 8447ef9c4..7825e9735 100644 --- a/modules/worker/front/routes.json +++ b/modules/worker/front/routes.json @@ -62,7 +62,8 @@ "url" : "/log", "state": "worker.card.workerLog", "component": "vn-worker-log", - "description": "Log" + "description": "Log", + "acl": ["hr"] }, { "url": "/pbx", "state": "worker.card.pbx", From 8384918d9ee91751e29b558375d8b58a233acff0 Mon Sep 17 00:00:00 2001 From: Joan Sanchez Date: Tue, 26 May 2020 14:42:45 +0200 Subject: [PATCH 2/4] Removed line --- loopback/common/models/loggable.js | 1 - 1 file changed, 1 deletion(-) diff --git a/loopback/common/models/loggable.js b/loopback/common/models/loggable.js index 61a571607..fac02ed1c 100644 --- a/loopback/common/models/loggable.js +++ b/loopback/common/models/loggable.js @@ -2,7 +2,6 @@ const pick = require('object.pick'); const LoopBackContext = require('loopback-context'); module.exports = function(Self) { - // const relations = ctx.Model.relations; Self.setup = function() { Self.super_.setup.call(this); }; From 3cd4ab086dce3969a125ad979389b6a29d7373d7 Mon Sep 17 00:00:00 2001 From: Joan Sanchez Date: Tue, 26 May 2020 15:49:20 +0200 Subject: [PATCH 3/4] Fix --- loopback/common/models/loggable.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/loopback/common/models/loggable.js b/loopback/common/models/loggable.js index fac02ed1c..ec6989717 100644 --- a/loopback/common/models/loggable.js +++ b/loopback/common/models/loggable.js @@ -13,6 +13,7 @@ module.exports = function(Self) { Self.observe('before save', async function(ctx) { const appModels = ctx.Model.app.models; + const definition = ctx.Model.definition; const options = {}; // Check for transactions @@ -29,7 +30,7 @@ module.exports = function(Self) { if (ctx.where && !ctx.currentInstance) { const fields = Object.keys(ctx.data); - const modelName = modelDef.name; + const modelName = definition.name; ctx.oldInstances = await appModels[modelName].find({ where: ctx.where, @@ -86,7 +87,7 @@ module.exports = function(Self) { async function logDeletedInstances(ctx, loopBackContext) { const appModels = ctx.Model.app.models; - const modelDef = ctx.Model.definition; + const definition = ctx.Model.definition; let options = {}; if (ctx.options && ctx.options.transaction) options.transaction = ctx.options.transaction; @@ -96,14 +97,12 @@ module.exports = function(Self) { if (loopBackContext) userFk = loopBackContext.active.accessToken.userId; - let definition = modelDef; - let changedModelValue = definition.settings.log.changedModelValue; let logRecord = { originFk: instance.originFk, userFk: userFk, action: 'delete', - changedModel: modelDef.name, + changedModel: definition.name, changedModelId: instance.id, changedModelValue: instance[changedModelValue], oldInstance: instance, From 5c3e8e94655be123907665e991eb2c42dbf68bf9 Mon Sep 17 00:00:00 2001 From: Joan Sanchez Date: Thu, 28 May 2020 08:09:12 +0200 Subject: [PATCH 4/4] Added insert ignore --- db/changes/10180-holyWeek/00-ACL.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/changes/10180-holyWeek/00-ACL.sql b/db/changes/10180-holyWeek/00-ACL.sql index d4b961e22..b0ab68a97 100644 --- a/db/changes/10180-holyWeek/00-ACL.sql +++ b/db/changes/10180-holyWeek/00-ACL.sql @@ -1 +1 @@ -INSERT INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) VALUES ('WorkerLog', '*', 'READ', 'ALLOW', 'ROLE', 'hr'); \ No newline at end of file +INSERT IGNORE INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) VALUES ('WorkerLog', '*', 'READ', 'ALLOW', 'ROLE', 'hr'); \ No newline at end of file