From 4d3d38ebe301bdd62a7cc252ab7bbbb8cba7a3ce Mon Sep 17 00:00:00 2001 From: alexm Date: Wed, 18 Oct 2023 15:22:04 +0200 Subject: [PATCH 1/7] refs #6014 feat: executeRoutine --- .../methods/application/executeRoutine.js | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 loopback/common/methods/application/executeRoutine.js diff --git a/loopback/common/methods/application/executeRoutine.js b/loopback/common/methods/application/executeRoutine.js new file mode 100644 index 0000000000..a5557bc617 --- /dev/null +++ b/loopback/common/methods/application/executeRoutine.js @@ -0,0 +1,61 @@ +module.exports = Self => { + Self.remoteMethodCtx('executeRoutine', { + description: 'Return the routes by worker', + accessType: '*', + accepts: [ + { + arg: 'routine', + type: 'string', + description: 'The routine sql', + required: true, + http: {source: 'path'} + }, + { + arg: 'params', + type: ['any'], + description: 'The array of params', + required: true, + } + ], + returns: { + type: 'any', + root: true + }, + http: { + path: `/:routine/execute-routine`, + verb: 'POST' + } + }); + + Self.executeRoutine = async(ctx, routine, params, options) => { + const userId = ctx.req.accessToken.userId; + + const myOptions = {}; + if (typeof options == 'object') + Object.assign(myOptions, options); + + const user = await Self.app.models.VnUser.findById(userId, { + fields: ['id', 'roleFk'], + include: { + relation: 'role', + scope: { + fields: ['id', 'name'] + } + } + }); + + const inherits = await Self.app.models.RoleRole.find({ + where: { + + } + }); + console.log(user.role.name); + + const checkACL = await models.ACL.checkAccessAcl(ctx, 'Application', routine, '*'); + if (!checkACL) throw error; + + const requestParams = [routine]; + requestParams.concat(params); + return Self.app.models.Route.rawSql(`CALL ?(...)`, requestParams, myOptions); + }; +}; From 483526c9702fe03ddd2fcc18586cb6b62cc36102 Mon Sep 17 00:00:00 2001 From: alexm Date: Mon, 23 Oct 2023 15:03:05 +0200 Subject: [PATCH 2/7] refs #6015 feat(executeRoutine): check db restrictions. test: add executeRoutine tests --- db/changes/234201/00-ACL_executeRoutine.sql | 3 + .../methods/application/executeRoutine.js | 67 +++++++-- .../application/spec/executeRoutine.spec.js | 138 ++++++++++++++++++ loopback/common/models/application.js | 1 + loopback/common/models/procs-priv.json | 44 ++++++ loopback/server/model-config.json | 10 +- 6 files changed, 248 insertions(+), 15 deletions(-) create mode 100644 db/changes/234201/00-ACL_executeRoutine.sql create mode 100644 loopback/common/methods/application/spec/executeRoutine.spec.js create mode 100644 loopback/common/models/procs-priv.json diff --git a/db/changes/234201/00-ACL_executeRoutine.sql b/db/changes/234201/00-ACL_executeRoutine.sql new file mode 100644 index 0000000000..dd112171a3 --- /dev/null +++ b/db/changes/234201/00-ACL_executeRoutine.sql @@ -0,0 +1,3 @@ +INSERT INTO `salix`.`ACL` (model, property, accessType, permission, principalType, principalId) + VALUES + ('Application', 'executeRoutine', '*', 'ALLOW', 'ROLE', 'employee'); diff --git a/loopback/common/methods/application/executeRoutine.js b/loopback/common/methods/application/executeRoutine.js index a5557bc617..eed34b3441 100644 --- a/loopback/common/methods/application/executeRoutine.js +++ b/loopback/common/methods/application/executeRoutine.js @@ -1,3 +1,5 @@ +const UserError = require('vn-loopback/util/user-error'); + module.exports = Self => { Self.remoteMethodCtx('executeRoutine', { description: 'Return the routes by worker', @@ -6,15 +8,24 @@ module.exports = Self => { { arg: 'routine', type: 'string', - description: 'The routine sql', + description: 'The routine name', required: true, http: {source: 'path'} }, { arg: 'params', type: ['any'], - description: 'The array of params', - required: true, + description: 'The params array', + }, + { + arg: 'schema', + type: 'string', + description: 'The routine schema', + }, + { + arg: 'type', + type: 'string', + description: 'The routine type', } ], returns: { @@ -27,14 +38,23 @@ module.exports = Self => { } }); - Self.executeRoutine = async(ctx, routine, params, options) => { + Self.executeRoutine = async(ctx, routine, params, schema, type, options) => { const userId = ctx.req.accessToken.userId; + const models = Self.app.models; + const isFunction = type == 'function'; + params = params ?? []; + schema = schema ?? 'vn'; + type = type ?? 'procedure'; + let caller = 'CALL'; - const myOptions = {}; + if (isFunction) + caller = 'SELECT'; + + const myOptions = {userId: ctx.req.accessToken.userId}; if (typeof options == 'object') Object.assign(myOptions, options); - const user = await Self.app.models.VnUser.findById(userId, { + const user = await models.VnUser.findById(userId, { fields: ['id', 'roleFk'], include: { relation: 'role', @@ -44,18 +64,37 @@ module.exports = Self => { } }); - const inherits = await Self.app.models.RoleRole.find({ + const inherits = await models.RoleRole.find({ + include: { + relation: 'inherits', + scope: { + fields: ['id', 'name'] + } + }, where: { - + role: user.role().id } }); - console.log(user.role.name); - const checkACL = await models.ACL.checkAccessAcl(ctx, 'Application', routine, '*'); - if (!checkACL) throw error; + const roles = inherits.map(inherit => inherit.inherits().name); - const requestParams = [routine]; - requestParams.concat(params); - return Self.app.models.Route.rawSql(`CALL ?(...)`, requestParams, myOptions); + const canExecute = await models.ProcsPriv.findOne({ + where: { + schema, + type: type.toUpperCase(), + name: routine, + host: process.env.NODE_ENV ? '' : '%', + role: {inq: roles} + } + }); + + if (!canExecute) throw new UserError(`You don't have enough privileges`, 'ACCESS_DENIED'); + + let argString = params.map(() => '?').join(','); + + const query = `${caller} ${schema}.${routine}(${argString})`; + + const [response] = await models.ProcsPriv.rawSql(query, params, myOptions); + return isFunction ? Object.values(response)[0] : response; }; }; diff --git a/loopback/common/methods/application/spec/executeRoutine.spec.js b/loopback/common/methods/application/spec/executeRoutine.spec.js new file mode 100644 index 0000000000..150c5d416d --- /dev/null +++ b/loopback/common/methods/application/spec/executeRoutine.spec.js @@ -0,0 +1,138 @@ +const models = require('vn-loopback/server/server').models; + +describe('Application executeRoutine()', () => { + const userWithoutPrivileges = 1; + const userWithPrivileges = 9; + const userWithInheritedPrivileges = 120; + let tx; + + function getCtx(userId) { + return { + req: { + accessToken: {userId}, + headers: {origin: 'http://localhost'} + } + }; + } + + beforeEach(async() => { + tx = await models.Application.beginTransaction({}); + const options = {transaction: tx}; + + await models.Application.rawSql(` + CREATE OR REPLACE PROCEDURE vn.myProcedure(vMyParam INT) + BEGIN + SELECT vMyParam myParam, t.* + FROM ticket t + LIMIT 2; + END + `, null, options); + + await models.Application.rawSql(` + CREATE OR REPLACE FUNCTION bs.myFunction(vMyParam INT) RETURNS int(11) + BEGIN + RETURN vMyParam; + END + `, null, options); + + await models.Application.rawSql(` + GRANT EXECUTE ON PROCEDURE vn.myProcedure TO developer; + GRANT EXECUTE ON FUNCTION bs.myFunction TO developer; + `, null, options); + }); + + it('should throw error when execute procedure and not have privileges', async() => { + const ctx = getCtx(userWithoutPrivileges); + + let error; + try { + const options = {transaction: tx}; + + await models.Application.executeRoutine( + ctx, + 'myProcedure', + [1], + null, + null, + options + ); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + error = e; + } + + expect(error.message).toEqual(`You don't have enough privileges`); + }); + + it('should execute procedure and get data', async() => { + const ctx = getCtx(userWithPrivileges); + try { + const options = {transaction: tx}; + + const response = await models.Application.executeRoutine( + ctx, + 'myProcedure', + [1], + null, + null, + options + ); + + expect(response.length).toEqual(2); + expect(response[0].myParam).toEqual(1); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should execute function and get data', async() => { + const ctx = getCtx(userWithPrivileges); + try { + const options = {transaction: tx}; + + const response = await models.Application.executeRoutine( + ctx, + 'myFunction', + [1], + 'bs', + 'function', + options + ); + + expect(response).toEqual(1); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should execute function and get data with user with inherited privileges', async() => { + const ctx = getCtx(userWithInheritedPrivileges); + try { + const options = {transaction: tx}; + + const response = await models.Application.executeRoutine( + ctx, + 'myFunction', + [1], + 'bs', + 'function', + options + ); + + expect(response).toEqual(1); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); +}); diff --git a/loopback/common/models/application.js b/loopback/common/models/application.js index 5e767fdc11..b9e639b1be 100644 --- a/loopback/common/models/application.js +++ b/loopback/common/models/application.js @@ -2,4 +2,5 @@ module.exports = function(Self) { require('../methods/application/status')(Self); require('../methods/application/post')(Self); + require('../methods/application/executeRoutine')(Self); }; diff --git a/loopback/common/models/procs-priv.json b/loopback/common/models/procs-priv.json new file mode 100644 index 0000000000..25221d586c --- /dev/null +++ b/loopback/common/models/procs-priv.json @@ -0,0 +1,44 @@ +{ + "name": "ProcsPriv", + "base": "VnModel", + "options": { + "mysql": { + "table": "mysql.procs_priv" + } + }, + "properties": { + "name": { + "id": 1, + "type": "string", + "mysql": { + "columnName": "Routine_name" + } + }, + "schema": { + "id": 3, + "type": "string", + "mysql": { + "columnName": "Db" + } + }, + "role": { + "type": "string", + "mysql": { + "columnName": "user" + } + }, + "type": { + "id": 2, + "type": "string", + "mysql": { + "columnName": "Routine_type" + } + }, + "host": { + "type": "string", + "mysql": { + "columnName": "Host" + } + } + } +} diff --git a/loopback/server/model-config.json b/loopback/server/model-config.json index 52b539f60e..33ef3797d6 100644 --- a/loopback/server/model-config.json +++ b/loopback/server/model-config.json @@ -49,5 +49,13 @@ }, "Container": { "dataSource": "vn" + }, + "ProcsPriv": { + "dataSource": "vn", + "options": { + "mysql": { + "table": "mysql.procs_priv" + } + } } -} \ No newline at end of file +} From 69255fe2fcacd1dc716b26dcf0c60b8face6882d Mon Sep 17 00:00:00 2001 From: alexm Date: Fri, 10 Nov 2023 10:58:58 +0100 Subject: [PATCH 3/7] refs #6014 feat(execute): use user_hasRoutinePriv. feat(execute): split in executeProc & executeFunc --- .vscode/settings.json | 3 +- db/changes/234201/00-ACL_executeRoutine.sql | 3 - db/changes/234801/00-ACL_executeRoutine.sql | 4 + db/dump/structure.sql | 84 ++++++++++++++ .../common/methods/application/execute.js | 34 ++++++ .../common/methods/application/executeFunc.js | 38 +++++++ .../common/methods/application/executeProc.js | 36 ++++++ .../methods/application/executeRoutine.js | 100 ----------------- ...executeRoutine.spec.js => execute.spec.js} | 103 +++++++++++------- loopback/common/models/application.js | 4 +- 10 files changed, 265 insertions(+), 144 deletions(-) delete mode 100644 db/changes/234201/00-ACL_executeRoutine.sql create mode 100644 db/changes/234801/00-ACL_executeRoutine.sql create mode 100644 loopback/common/methods/application/execute.js create mode 100644 loopback/common/methods/application/executeFunc.js create mode 100644 loopback/common/methods/application/executeProc.js delete mode 100644 loopback/common/methods/application/executeRoutine.js rename loopback/common/methods/application/spec/{executeRoutine.spec.js => execute.spec.js} (51%) diff --git a/.vscode/settings.json b/.vscode/settings.json index 9ed1c8fc2a..899dfc7884 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -12,6 +12,7 @@ "editor.defaultFormatter": "dbaeumer.vscode-eslint" }, "cSpell.words": [ - "salix" + "salix", + "fdescribe" ] } diff --git a/db/changes/234201/00-ACL_executeRoutine.sql b/db/changes/234201/00-ACL_executeRoutine.sql deleted file mode 100644 index dd112171a3..0000000000 --- a/db/changes/234201/00-ACL_executeRoutine.sql +++ /dev/null @@ -1,3 +0,0 @@ -INSERT INTO `salix`.`ACL` (model, property, accessType, permission, principalType, principalId) - VALUES - ('Application', 'executeRoutine', '*', 'ALLOW', 'ROLE', 'employee'); diff --git a/db/changes/234801/00-ACL_executeRoutine.sql b/db/changes/234801/00-ACL_executeRoutine.sql new file mode 100644 index 0000000000..cfe7018e91 --- /dev/null +++ b/db/changes/234801/00-ACL_executeRoutine.sql @@ -0,0 +1,4 @@ +INSERT INTO `salix`.`ACL` (model, property, accessType, permission, principalType, principalId) + VALUES + ('Application', 'executeProc', '*', 'ALLOW', 'ROLE', 'employee'), + ('Application', 'executeFunc', '*', 'ALLOW', 'ROLE', 'employee'); diff --git a/db/dump/structure.sql b/db/dump/structure.sql index b242821fc6..a8280dc1db 100644 --- a/db/dump/structure.sql +++ b/db/dump/structure.sql @@ -2352,6 +2352,90 @@ BEGIN END IF; END ;; DELIMITER ; + + +DELIMITER ;; +CREATE DEFINER=`root`@`localhost` FUNCTION `account`.`user_hasRoutinePriv`(vType ENUM('PROCEDURE', 'FUNCTION'), + vChain VARCHAR(100), + vUserFk INT +) RETURNS tinyint(1) + READS SQL DATA +BEGIN + +/** + * Search if the user has privileges on routines. + * + * @param vType procedure or function + * @param vChain string passed with this syntax dbName.tableName + * @param vUserFk user to ckeck + * @return vHasPrivilege + */ + DECLARE vHasPrivilege BOOL DEFAULT FALSE; + DECLARE vDb VARCHAR(50); + DECLARE vObject VARCHAR(50); + DECLARE vChainExists BOOL; + DECLARE vExecutePriv INT DEFAULT 262144; + -- 262144 = CONV(1000000000000000000, 2, 10) + -- 1000000000000000000 execution permission expressed in binary base + + SET vDb = SUBSTRING_INDEX(vChain, '.', 1); + SET vChain = SUBSTRING(vChain, LENGTH(vDb) + 2); + SET vObject = SUBSTRING_INDEX(vChain, '.', 1); + + SELECT COUNT(*) INTO vChainExists + FROM mysql.proc + WHERE db = vDb + AND `name` = vObject + AND `type` = vType + LIMIT 1; + + IF NOT vChainExists THEN + RETURN FALSE; + END IF; + + DROP TEMPORARY TABLE IF EXISTS tRole; + CREATE TEMPORARY TABLE tRole + (INDEX (`name`)) + ENGINE = MEMORY + SELECT r.`name` + FROM user u + JOIN roleRole rr ON rr.role = u.role + JOIN `role` r ON r.id = rr.inheritsFrom + WHERE u.id = vUserFk; + + SELECT TRUE INTO vHasPrivilege + FROM mysql.global_priv gp + JOIN tRole tr ON tr.name = gp.`User` + OR CONCAT('$', tr.name) = gp.`User` + WHERE JSON_VALUE(gp.Priv, '$.access') >= vExecutePriv + AND gp.Host = '' + LIMIT 1; + + IF NOT vHasPrivilege THEN + SELECT TRUE INTO vHasPrivilege + FROM mysql.db db + JOIN tRole tr ON tr.name = db.`User` + WHERE db.Db = vDb + AND db.Execute_priv = 'Y'; + END IF; + + IF NOT vHasPrivilege THEN + SELECT TRUE INTO vHasPrivilege + FROM mysql.procs_priv pp + JOIN tRole tr ON tr.name = pp.`User` + WHERE pp.Db = vDb + AND pp.Routine_name = vObject + AND pp.Routine_type = vType + AND pp.Proc_priv = 'Execute' + LIMIT 1; + END IF; + + DROP TEMPORARY TABLE tRole; + RETURN vHasPrivilege; +END ;; +DELIMITER ; + + /*!50003 SET sql_mode = @saved_sql_mode */ ; /*!50003 SET character_set_client = @saved_cs_client */ ; /*!50003 SET character_set_results = @saved_cs_results */ ; diff --git a/loopback/common/methods/application/execute.js b/loopback/common/methods/application/execute.js new file mode 100644 index 0000000000..7a24df0d40 --- /dev/null +++ b/loopback/common/methods/application/execute.js @@ -0,0 +1,34 @@ +const UserError = require('vn-loopback/util/user-error'); + +module.exports = Self => { + Self.execute = async(ctx, routine, params, schema, type, options) => { + const userId = ctx.req.accessToken.userId; + const models = Self.app.models; + let caller = 'CALL'; + + params = params ?? []; + schema = schema ?? 'vn'; + type = type ?? 'procedure'; + + const myOptions = {userId: ctx.req.accessToken.userId}; + if (typeof options == 'object') + Object.assign(myOptions, options); + + const chain = `${schema}.${routine}`; + const [canExecute] = await models.ProcsPriv.rawSql( + 'SELECT account.user_hasRoutinePriv(?,?,?)', + [type.toUpperCase(), chain, userId], + myOptions); + if (!Object.values(canExecute)[0]) throw new UserError(`You don't have enough privileges`, 'ACCESS_DENIED'); + + const isFunction = type == 'function'; + let argString = params.map(() => '?').join(','); + + if (isFunction) + caller = 'SELECT'; + const query = `${caller} ${chain}(${argString})`; + + const [response] = await models.ProcsPriv.rawSql(query, params, myOptions); + return response; + }; +}; diff --git a/loopback/common/methods/application/executeFunc.js b/loopback/common/methods/application/executeFunc.js new file mode 100644 index 0000000000..0a90e8639b --- /dev/null +++ b/loopback/common/methods/application/executeFunc.js @@ -0,0 +1,38 @@ +module.exports = Self => { + Self.remoteMethodCtx('executeFunc', { + description: 'Return result of function', + accessType: '*', + accepts: [ + { + arg: 'routine', + type: 'string', + description: 'The routine name', + required: true, + http: {source: 'path'} + }, + { + arg: 'params', + type: ['any'], + description: 'The params array', + }, + { + arg: 'schema', + type: 'string', + description: 'The routine schema', + } + ], + returns: { + type: 'any', + root: true + }, + http: { + path: `/:routine/execute-func`, + verb: 'POST' + } + }); + + Self.executeFunc = async(ctx, routine, params, schema, options) => { + const response = await Self.execute(ctx, routine, params, schema, 'function', options); + return Object.values(response)[0]; + }; +}; diff --git a/loopback/common/methods/application/executeProc.js b/loopback/common/methods/application/executeProc.js new file mode 100644 index 0000000000..635944eb74 --- /dev/null +++ b/loopback/common/methods/application/executeProc.js @@ -0,0 +1,36 @@ +module.exports = Self => { + Self.remoteMethodCtx('executeProc', { + description: 'Return result of procedure', + accessType: '*', + accepts: [ + { + arg: 'routine', + type: 'string', + description: 'The routine name', + required: true, + http: {source: 'path'} + }, + { + arg: 'params', + type: ['any'], + description: 'The params array', + }, + { + arg: 'schema', + type: 'string', + description: 'The routine schema', + } + ], + returns: { + type: 'any', + root: true + }, + http: { + path: `/:routine/execute-proc`, + verb: 'POST' + } + }); + + Self.executeProc = async(ctx, routine, params, schema, options) => + Self.execute(ctx, routine, params, schema, 'procedure', options); +}; diff --git a/loopback/common/methods/application/executeRoutine.js b/loopback/common/methods/application/executeRoutine.js deleted file mode 100644 index eed34b3441..0000000000 --- a/loopback/common/methods/application/executeRoutine.js +++ /dev/null @@ -1,100 +0,0 @@ -const UserError = require('vn-loopback/util/user-error'); - -module.exports = Self => { - Self.remoteMethodCtx('executeRoutine', { - description: 'Return the routes by worker', - accessType: '*', - accepts: [ - { - arg: 'routine', - type: 'string', - description: 'The routine name', - required: true, - http: {source: 'path'} - }, - { - arg: 'params', - type: ['any'], - description: 'The params array', - }, - { - arg: 'schema', - type: 'string', - description: 'The routine schema', - }, - { - arg: 'type', - type: 'string', - description: 'The routine type', - } - ], - returns: { - type: 'any', - root: true - }, - http: { - path: `/:routine/execute-routine`, - verb: 'POST' - } - }); - - Self.executeRoutine = async(ctx, routine, params, schema, type, options) => { - const userId = ctx.req.accessToken.userId; - const models = Self.app.models; - const isFunction = type == 'function'; - params = params ?? []; - schema = schema ?? 'vn'; - type = type ?? 'procedure'; - let caller = 'CALL'; - - if (isFunction) - caller = 'SELECT'; - - const myOptions = {userId: ctx.req.accessToken.userId}; - if (typeof options == 'object') - Object.assign(myOptions, options); - - const user = await models.VnUser.findById(userId, { - fields: ['id', 'roleFk'], - include: { - relation: 'role', - scope: { - fields: ['id', 'name'] - } - } - }); - - const inherits = await models.RoleRole.find({ - include: { - relation: 'inherits', - scope: { - fields: ['id', 'name'] - } - }, - where: { - role: user.role().id - } - }); - - const roles = inherits.map(inherit => inherit.inherits().name); - - const canExecute = await models.ProcsPriv.findOne({ - where: { - schema, - type: type.toUpperCase(), - name: routine, - host: process.env.NODE_ENV ? '' : '%', - role: {inq: roles} - } - }); - - if (!canExecute) throw new UserError(`You don't have enough privileges`, 'ACCESS_DENIED'); - - let argString = params.map(() => '?').join(','); - - const query = `${caller} ${schema}.${routine}(${argString})`; - - const [response] = await models.ProcsPriv.rawSql(query, params, myOptions); - return isFunction ? Object.values(response)[0] : response; - }; -}; diff --git a/loopback/common/methods/application/spec/executeRoutine.spec.js b/loopback/common/methods/application/spec/execute.spec.js similarity index 51% rename from loopback/common/methods/application/spec/executeRoutine.spec.js rename to loopback/common/methods/application/spec/execute.spec.js index 150c5d416d..26e648531f 100644 --- a/loopback/common/methods/application/spec/executeRoutine.spec.js +++ b/loopback/common/methods/application/spec/execute.spec.js @@ -1,6 +1,6 @@ const models = require('vn-loopback/server/server').models; -describe('Application executeRoutine()', () => { +describe('Application execute()/executeProc()/executeFunc()', () => { const userWithoutPrivileges = 1; const userWithPrivileges = 9; const userWithInheritedPrivileges = 120; @@ -48,7 +48,7 @@ describe('Application executeRoutine()', () => { try { const options = {transaction: tx}; - await models.Application.executeRoutine( + await models.Application.execute( ctx, 'myProcedure', [1], @@ -71,7 +71,7 @@ describe('Application executeRoutine()', () => { try { const options = {transaction: tx}; - const response = await models.Application.executeRoutine( + const response = await models.Application.execute( ctx, 'myProcedure', [1], @@ -90,49 +90,74 @@ describe('Application executeRoutine()', () => { } }); - it('should execute function and get data', async() => { - const ctx = getCtx(userWithPrivileges); - try { - const options = {transaction: tx}; + describe('Application executeProc()', () => { + it('should execute procedure and get data (executeProc)', async() => { + const ctx = getCtx(userWithPrivileges); + try { + const options = {transaction: tx}; - const response = await models.Application.executeRoutine( - ctx, - 'myFunction', - [1], - 'bs', - 'function', - options - ); + const response = await models.Application.executeProc( + ctx, + 'myProcedure', + [1], + null, + options + ); - expect(response).toEqual(1); + expect(response.length).toEqual(2); + expect(response[0].myParam).toEqual(1); - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); }); - it('should execute function and get data with user with inherited privileges', async() => { - const ctx = getCtx(userWithInheritedPrivileges); - try { - const options = {transaction: tx}; + describe('Application executeFunc()', () => { + it('should execute function and get data', async() => { + const ctx = getCtx(userWithPrivileges); + try { + const options = {transaction: tx}; - const response = await models.Application.executeRoutine( - ctx, - 'myFunction', - [1], - 'bs', - 'function', - options - ); + const response = await models.Application.executeFunc( + ctx, + 'myFunction', + [1], + 'bs', + options + ); - expect(response).toEqual(1); + expect(response).toEqual(1); - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should execute function and get data with user with inherited privileges', async() => { + const ctx = getCtx(userWithInheritedPrivileges); + try { + const options = {transaction: tx}; + + const response = await models.Application.executeFunc( + ctx, + 'myFunction', + [1], + 'bs', + options + ); + + expect(response).toEqual(1); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); }); }); diff --git a/loopback/common/models/application.js b/loopback/common/models/application.js index b9e639b1be..ac8ae78f0d 100644 --- a/loopback/common/models/application.js +++ b/loopback/common/models/application.js @@ -2,5 +2,7 @@ module.exports = function(Self) { require('../methods/application/status')(Self); require('../methods/application/post')(Self); - require('../methods/application/executeRoutine')(Self); + require('../methods/application/execute')(Self); + require('../methods/application/executeProc')(Self); + require('../methods/application/executeFunc')(Self); }; From 59d2da24eb6d5329b0700453abaabb3215105f79 Mon Sep 17 00:00:00 2001 From: alexm Date: Mon, 13 Nov 2023 09:36:20 +0100 Subject: [PATCH 4/7] refs #6014 refactor(execute): split code --- .../common/methods/application/execute.js | 19 +++++++------------ .../common/methods/application/executeFunc.js | 6 +++++- .../common/methods/application/executeProc.js | 9 +++++++-- .../methods/application/spec/execute.spec.js | 8 ++------ 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/loopback/common/methods/application/execute.js b/loopback/common/methods/application/execute.js index 7a24df0d40..c0475dbfed 100644 --- a/loopback/common/methods/application/execute.js +++ b/loopback/common/methods/application/execute.js @@ -1,32 +1,27 @@ const UserError = require('vn-loopback/util/user-error'); module.exports = Self => { - Self.execute = async(ctx, routine, params, schema, type, options) => { + Self.execute = async(ctx, query, params, options) => { const userId = ctx.req.accessToken.userId; const models = Self.app.models; - let caller = 'CALL'; - params = params ?? []; - schema = schema ?? 'vn'; - type = type ?? 'procedure'; const myOptions = {userId: ctx.req.accessToken.userId}; if (typeof options == 'object') Object.assign(myOptions, options); - const chain = `${schema}.${routine}`; + let [caller, chain] = query.split(' '); + if (!chain.includes('.')) chain = 'vn.' + chain; + const [canExecute] = await models.ProcsPriv.rawSql( 'SELECT account.user_hasRoutinePriv(?,?,?)', - [type.toUpperCase(), chain, userId], + [caller == 'CALL' ? 'PROCEDURE' : 'FUNCTION', chain, userId], myOptions); + if (!Object.values(canExecute)[0]) throw new UserError(`You don't have enough privileges`, 'ACCESS_DENIED'); - const isFunction = type == 'function'; let argString = params.map(() => '?').join(','); - - if (isFunction) - caller = 'SELECT'; - const query = `${caller} ${chain}(${argString})`; + query = `${query}(${argString})`; const [response] = await models.ProcsPriv.rawSql(query, params, myOptions); return response; diff --git a/loopback/common/methods/application/executeFunc.js b/loopback/common/methods/application/executeFunc.js index 0a90e8639b..49e2cdc21c 100644 --- a/loopback/common/methods/application/executeFunc.js +++ b/loopback/common/methods/application/executeFunc.js @@ -32,7 +32,11 @@ module.exports = Self => { }); Self.executeFunc = async(ctx, routine, params, schema, options) => { - const response = await Self.execute(ctx, routine, params, schema, 'function', options); + if (schema) + routine = schema + '.' + routine; + + const query = `SELECT ${routine}`; + const response = await Self.execute(ctx, query, params, options); return Object.values(response)[0]; }; }; diff --git a/loopback/common/methods/application/executeProc.js b/loopback/common/methods/application/executeProc.js index 635944eb74..524831e86d 100644 --- a/loopback/common/methods/application/executeProc.js +++ b/loopback/common/methods/application/executeProc.js @@ -31,6 +31,11 @@ module.exports = Self => { } }); - Self.executeProc = async(ctx, routine, params, schema, options) => - Self.execute(ctx, routine, params, schema, 'procedure', options); + Self.executeProc = async(ctx, routine, params, schema, options) => { + if (schema) + routine = schema + '.' + routine; + + const query = `CALL ${routine}`; + return Self.execute(ctx, query, params, options); + }; }; diff --git a/loopback/common/methods/application/spec/execute.spec.js b/loopback/common/methods/application/spec/execute.spec.js index 26e648531f..4c57279847 100644 --- a/loopback/common/methods/application/spec/execute.spec.js +++ b/loopback/common/methods/application/spec/execute.spec.js @@ -50,10 +50,8 @@ describe('Application execute()/executeProc()/executeFunc()', () => { await models.Application.execute( ctx, - 'myProcedure', + 'CALL myProcedure', [1], - null, - null, options ); @@ -73,10 +71,8 @@ describe('Application execute()/executeProc()/executeFunc()', () => { const response = await models.Application.execute( ctx, - 'myProcedure', + 'CALL myProcedure', [1], - null, - null, options ); From cce61ae8ccf9b7d747a0f934a187b9adcfb8bc59 Mon Sep 17 00:00:00 2001 From: alexm Date: Tue, 14 Nov 2023 09:17:46 +0100 Subject: [PATCH 5/7] refs #6014 refactor(execute): schema required --- .../common/methods/application/execute.js | 12 +++++------ .../common/methods/application/executeFunc.js | 19 +++++++++--------- .../common/methods/application/executeProc.js | 20 +++++++++---------- .../methods/application/spec/execute.spec.js | 12 ++++++----- 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/loopback/common/methods/application/execute.js b/loopback/common/methods/application/execute.js index c0475dbfed..7995b12e35 100644 --- a/loopback/common/methods/application/execute.js +++ b/loopback/common/methods/application/execute.js @@ -1,7 +1,7 @@ const UserError = require('vn-loopback/util/user-error'); module.exports = Self => { - Self.execute = async(ctx, query, params, options) => { + Self.execute = async(ctx, type, query, params, options) => { const userId = ctx.req.accessToken.userId; const models = Self.app.models; params = params ?? []; @@ -10,20 +10,18 @@ module.exports = Self => { if (typeof options == 'object') Object.assign(myOptions, options); - let [caller, chain] = query.split(' '); - if (!chain.includes('.')) chain = 'vn.' + chain; + const chain = query.split(' ')[1]; const [canExecute] = await models.ProcsPriv.rawSql( 'SELECT account.user_hasRoutinePriv(?,?,?)', - [caller == 'CALL' ? 'PROCEDURE' : 'FUNCTION', chain, userId], + [type, chain, userId], myOptions); if (!Object.values(canExecute)[0]) throw new UserError(`You don't have enough privileges`, 'ACCESS_DENIED'); - let argString = params.map(() => '?').join(','); - query = `${query}(${argString})`; + const argString = params.map(() => '?').join(','); - const [response] = await models.ProcsPriv.rawSql(query, params, myOptions); + const [response] = await models.ProcsPriv.rawSql(query + `(${argString})`, params, myOptions); return response; }; }; diff --git a/loopback/common/methods/application/executeFunc.js b/loopback/common/methods/application/executeFunc.js index 49e2cdc21c..f915f44829 100644 --- a/loopback/common/methods/application/executeFunc.js +++ b/loopback/common/methods/application/executeFunc.js @@ -10,16 +10,17 @@ module.exports = Self => { required: true, http: {source: 'path'} }, + { + arg: 'schema', + type: 'string', + description: 'The routine schema', + required: true, + }, { arg: 'params', type: ['any'], description: 'The params array', }, - { - arg: 'schema', - type: 'string', - description: 'The routine schema', - } ], returns: { type: 'any', @@ -31,12 +32,10 @@ module.exports = Self => { } }); - Self.executeFunc = async(ctx, routine, params, schema, options) => { - if (schema) - routine = schema + '.' + routine; + Self.executeFunc = async(ctx, routine, schema, params, options) => { + const query = `SELECT ${schema}.${routine}`; - const query = `SELECT ${routine}`; - const response = await Self.execute(ctx, query, params, options); + const response = await Self.execute(ctx, 'FUNCTION', query, params, options); return Object.values(response)[0]; }; }; diff --git a/loopback/common/methods/application/executeProc.js b/loopback/common/methods/application/executeProc.js index 524831e86d..5859611d98 100644 --- a/loopback/common/methods/application/executeProc.js +++ b/loopback/common/methods/application/executeProc.js @@ -10,16 +10,17 @@ module.exports = Self => { required: true, http: {source: 'path'} }, + { + arg: 'schema', + type: 'string', + description: 'The routine schema', + required: true, + }, { arg: 'params', type: ['any'], description: 'The params array', }, - { - arg: 'schema', - type: 'string', - description: 'The routine schema', - } ], returns: { type: 'any', @@ -31,11 +32,8 @@ module.exports = Self => { } }); - Self.executeProc = async(ctx, routine, params, schema, options) => { - if (schema) - routine = schema + '.' + routine; - - const query = `CALL ${routine}`; - return Self.execute(ctx, query, params, options); + Self.executeProc = async(ctx, routine, schema, params, options) => { + const query = `CALL ${schema}.${routine}`; + return Self.execute(ctx, 'PROCEDURE', query, params, options); }; }; diff --git a/loopback/common/methods/application/spec/execute.spec.js b/loopback/common/methods/application/spec/execute.spec.js index 4c57279847..1a0a8ace9b 100644 --- a/loopback/common/methods/application/spec/execute.spec.js +++ b/loopback/common/methods/application/spec/execute.spec.js @@ -50,7 +50,8 @@ describe('Application execute()/executeProc()/executeFunc()', () => { await models.Application.execute( ctx, - 'CALL myProcedure', + 'PROCEDURE', + 'CALL vn.myProcedure', [1], options ); @@ -71,7 +72,8 @@ describe('Application execute()/executeProc()/executeFunc()', () => { const response = await models.Application.execute( ctx, - 'CALL myProcedure', + 'PROCEDURE', + 'CALL vn.myProcedure', [1], options ); @@ -95,8 +97,8 @@ describe('Application execute()/executeProc()/executeFunc()', () => { const response = await models.Application.executeProc( ctx, 'myProcedure', + 'vn', [1], - null, options ); @@ -120,8 +122,8 @@ describe('Application execute()/executeProc()/executeFunc()', () => { const response = await models.Application.executeFunc( ctx, 'myFunction', - [1], 'bs', + [1], options ); @@ -142,8 +144,8 @@ describe('Application execute()/executeProc()/executeFunc()', () => { const response = await models.Application.executeFunc( ctx, 'myFunction', - [1], 'bs', + [1], options ); From aec1c9d4475bfb0605145b3617af9acf4ec522f2 Mon Sep 17 00:00:00 2001 From: alexm Date: Tue, 14 Nov 2023 09:54:07 +0100 Subject: [PATCH 6/7] fix: correct align --- modules/ticket/back/methods/ticket/closure.js | 288 +++++++++--------- 1 file changed, 144 insertions(+), 144 deletions(-) diff --git a/modules/ticket/back/methods/ticket/closure.js b/modules/ticket/back/methods/ticket/closure.js index 9f9aec9bd1..7a2825a4d6 100644 --- a/modules/ticket/back/methods/ticket/closure.js +++ b/modules/ticket/back/methods/ticket/closure.js @@ -5,177 +5,177 @@ const config = require('vn-print/core/config'); const storage = require('vn-print/core/storage'); module.exports = async function(ctx, Self, tickets, reqArgs = {}) { - const userId = ctx.req.accessToken.userId; - if (tickets.length == 0) return; + const userId = ctx.req.accessToken.userId; + if (tickets.length == 0) return; - const failedtickets = []; - for (const ticket of tickets) { - try { - await Self.rawSql(`CALL vn.ticket_closeByTicket(?)`, [ticket.id], {userId}); + const failedtickets = []; + for (const ticket of tickets) { + try { + await Self.rawSql(`CALL vn.ticket_closeByTicket(?)`, [ticket.id], {userId}); - const [invoiceOut] = await Self.rawSql(` - SELECT io.id, io.ref, io.serial, cny.code companyCode, io.issued - FROM ticket t - JOIN invoiceOut io ON io.ref = t.refFk - JOIN company cny ON cny.id = io.companyFk - WHERE t.id = ? - `, [ticket.id]); + const [invoiceOut] = await Self.rawSql(` + SELECT io.id, io.ref, io.serial, cny.code companyCode, io.issued + FROM ticket t + JOIN invoiceOut io ON io.ref = t.refFk + JOIN company cny ON cny.id = io.companyFk + WHERE t.id = ? + `, [ticket.id]); - const mailOptions = { - overrideAttachments: true, - attachments: [] - }; + const mailOptions = { + overrideAttachments: true, + attachments: [] + }; - const isToBeMailed = ticket.recipient && ticket.salesPersonFk && ticket.isToBeMailed; + const isToBeMailed = ticket.recipient && ticket.salesPersonFk && ticket.isToBeMailed; - if (invoiceOut) { - const args = { - reference: invoiceOut.ref, - recipientId: ticket.clientFk, - recipient: ticket.recipient, - replyTo: ticket.salesPersonEmail - }; + if (invoiceOut) { + const args = { + reference: invoiceOut.ref, + recipientId: ticket.clientFk, + recipient: ticket.recipient, + replyTo: ticket.salesPersonEmail + }; - const invoiceReport = new Report('invoice', args); - const stream = await invoiceReport.toPdfStream(); + const invoiceReport = new Report('invoice', args); + const stream = await invoiceReport.toPdfStream(); - const issued = invoiceOut.issued; - const year = issued.getFullYear().toString(); - const month = (issued.getMonth() + 1).toString(); - const day = issued.getDate().toString(); + const issued = invoiceOut.issued; + const year = issued.getFullYear().toString(); + const month = (issued.getMonth() + 1).toString(); + const day = issued.getDate().toString(); - const fileName = `${year}${invoiceOut.ref}.pdf`; + const fileName = `${year}${invoiceOut.ref}.pdf`; - // Store invoice - await storage.write(stream, { - type: 'invoice', - path: `${year}/${month}/${day}`, - fileName: fileName - }); + // Store invoice + await storage.write(stream, { + type: 'invoice', + path: `${year}/${month}/${day}`, + fileName: fileName + }); - await Self.rawSql('UPDATE invoiceOut SET hasPdf = true WHERE id = ?', [invoiceOut.id], {userId}); + await Self.rawSql('UPDATE invoiceOut SET hasPdf = true WHERE id = ?', [invoiceOut.id], {userId}); - if (isToBeMailed) { - const invoiceAttachment = { - filename: fileName, - content: stream - }; + if (isToBeMailed) { + const invoiceAttachment = { + filename: fileName, + content: stream + }; - if (invoiceOut.serial == 'E' && invoiceOut.companyCode == 'VNL') { - const exportation = new Report('exportation', args); - const stream = await exportation.toPdfStream(); - const fileName = `CITES-${invoiceOut.ref}.pdf`; + if (invoiceOut.serial == 'E' && invoiceOut.companyCode == 'VNL') { + const exportation = new Report('exportation', args); + const stream = await exportation.toPdfStream(); + const fileName = `CITES-${invoiceOut.ref}.pdf`; - mailOptions.attachments.push({ - filename: fileName, - content: stream - }); - } + mailOptions.attachments.push({ + filename: fileName, + content: stream + }); + } - mailOptions.attachments.push(invoiceAttachment); + mailOptions.attachments.push(invoiceAttachment); - const email = new Email('invoice', args); - await email.send(mailOptions); - } - } else if (isToBeMailed) { - const args = { - id: ticket.id, - recipientId: ticket.clientFk, - recipient: ticket.recipient, - replyTo: ticket.salesPersonEmail - }; + const email = new Email('invoice', args); + await email.send(mailOptions); + } + } else if (isToBeMailed) { + const args = { + id: ticket.id, + recipientId: ticket.clientFk, + recipient: ticket.recipient, + replyTo: ticket.salesPersonEmail + }; - const email = new Email('delivery-note-link', args); - await email.send(); - } + const email = new Email('delivery-note-link', args); + await email.send(); + } - // Incoterms authorization - const [{firstOrder}] = await Self.rawSql(` - SELECT COUNT(*) as firstOrder - FROM ticket t - JOIN client c ON c.id = t.clientFk - WHERE t.clientFk = ? - AND NOT t.isDeleted - AND c.isVies - `, [ticket.clientFk]); + // Incoterms authorization + const [{firstOrder}] = await Self.rawSql(` + SELECT COUNT(*) as firstOrder + FROM ticket t + JOIN client c ON c.id = t.clientFk + WHERE t.clientFk = ? + AND NOT t.isDeleted + AND c.isVies + `, [ticket.clientFk]); - if (firstOrder == 1) { - const args = { - id: ticket.clientFk, - companyId: ticket.companyFk, - recipientId: ticket.clientFk, - recipient: ticket.recipient, - replyTo: ticket.salesPersonEmail - }; + if (firstOrder == 1) { + const args = { + id: ticket.clientFk, + companyId: ticket.companyFk, + recipientId: ticket.clientFk, + recipient: ticket.recipient, + replyTo: ticket.salesPersonEmail + }; - const email = new Email('incoterms-authorization', args); - await email.send(); + const email = new Email('incoterms-authorization', args); + await email.send(); - const [sample] = await Self.rawSql( - `SELECT id - FROM sample - WHERE code = 'incoterms-authorization' - `); + const [sample] = await Self.rawSql( + `SELECT id + FROM sample + WHERE code = 'incoterms-authorization' + `); - await Self.rawSql(` - INSERT INTO clientSample (clientFk, typeFk, companyFk) VALUES(?, ?, ?) - `, [ticket.clientFk, sample.id, ticket.companyFk], {userId}); - }; - } catch (error) { - // Domain not found - if (error.responseCode == 450) - return invalidEmail(ticket); + await Self.rawSql(` + INSERT INTO clientSample (clientFk, typeFk, companyFk) VALUES(?, ?, ?) + `, [ticket.clientFk, sample.id, ticket.companyFk], {userId}); + } + } catch (error) { + // Domain not found + if (error.responseCode == 450) + return invalidEmail(ticket); - // Save tickets on a list of failed ids - failedtickets.push({ - id: ticket.id, - stacktrace: error - }); - } - } + // Save tickets on a list of failed ids + failedtickets.push({ + id: ticket.id, + stacktrace: error + }); + } + } - // Send email with failed tickets - if (failedtickets.length > 0) { - let body = 'This following tickets have failed:

'; + // Send email with failed tickets + if (failedtickets.length > 0) { + let body = 'This following tickets have failed:

'; - for (const ticket of failedtickets) { - body += `Ticket: ${ticket.id} -
${ticket.stacktrace}

`; - } + for (const ticket of failedtickets) { + body += `Ticket: ${ticket.id} +
${ticket.stacktrace}

`; + } - smtp.send({ - to: config.app.reportEmail, - subject: '[API] Nightly ticket closure report', - html: body - }); - } + smtp.send({ + to: config.app.reportEmail, + subject: '[API] Nightly ticket closure report', + html: body + }); + } - async function invalidEmail(ticket) { - await Self.rawSql(`UPDATE client SET email = NULL WHERE id = ?`, [ - ticket.clientFk - ], {userId}); + async function invalidEmail(ticket) { + await Self.rawSql(`UPDATE client SET email = NULL WHERE id = ?`, [ + ticket.clientFk + ], {userId}); - const oldInstance = `{"email": "${ticket.recipient}"}`; - const newInstance = `{"email": ""}`; - await Self.rawSql(` - INSERT INTO clientLog (originFk, userFk, action, changedModel, oldInstance, newInstance) - VALUES (?, NULL, 'UPDATE', 'Client', ?, ?)`, [ - ticket.clientFk, - oldInstance, - newInstance - ], {userId}); + const oldInstance = `{"email": "${ticket.recipient}"}`; + const newInstance = `{"email": ""}`; + await Self.rawSql(` + INSERT INTO clientLog (originFk, userFk, action, changedModel, oldInstance, newInstance) + VALUES (?, NULL, 'UPDATE', 'Client', ?, ?)`, [ + ticket.clientFk, + oldInstance, + newInstance + ], {userId}); - const body = `No se ha podido enviar el albarán ${ticket.id} - al cliente ${ticket.clientFk} - ${ticket.clientName} - porque la dirección de email "${ticket.recipient}" no es correcta - o no está disponible.

- Para evitar que se repita este error, se ha eliminado la dirección de email de la ficha del cliente. - Actualiza la dirección de email con una correcta.`; + const body = `No se ha podido enviar el albarán ${ticket.id} + al cliente ${ticket.clientFk} - ${ticket.clientName} + porque la dirección de email "${ticket.recipient}" no es correcta + o no está disponible.

+ Para evitar que se repita este error, se ha eliminado la dirección de email de la ficha del cliente. + Actualiza la dirección de email con una correcta.`; - smtp.send({ - to: ticket.salesPersonEmail, - subject: 'No se ha podido enviar el albarán', - html: body - }); - } + smtp.send({ + to: ticket.salesPersonEmail, + subject: 'No se ha podido enviar el albarán', + html: body + }); + } }; From 2bec1a2bed8fb261fe85ecf898467d78542de2ab Mon Sep 17 00:00:00 2001 From: sergiodt Date: Tue, 14 Nov 2023 14:02:40 +0100 Subject: [PATCH 7/7] refs #6014 fix: response type. fix: accessType EXECUTE --- loopback/common/methods/application/execute.js | 5 +++-- loopback/common/methods/application/executeFunc.js | 2 +- loopback/common/methods/application/executeProc.js | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/loopback/common/methods/application/execute.js b/loopback/common/methods/application/execute.js index 7995b12e35..a468dcd700 100644 --- a/loopback/common/methods/application/execute.js +++ b/loopback/common/methods/application/execute.js @@ -21,7 +21,8 @@ module.exports = Self => { const argString = params.map(() => '?').join(','); - const [response] = await models.ProcsPriv.rawSql(query + `(${argString})`, params, myOptions); - return response; + const response = await models.ProcsPriv.rawSql(query + `(${argString})`, params, myOptions); + if (!Array.isArray(response)) return; + return response[0]; }; }; diff --git a/loopback/common/methods/application/executeFunc.js b/loopback/common/methods/application/executeFunc.js index f915f44829..a42fdae677 100644 --- a/loopback/common/methods/application/executeFunc.js +++ b/loopback/common/methods/application/executeFunc.js @@ -1,7 +1,7 @@ module.exports = Self => { Self.remoteMethodCtx('executeFunc', { description: 'Return result of function', - accessType: '*', + accessType: 'EXECUTE', accepts: [ { arg: 'routine', diff --git a/loopback/common/methods/application/executeProc.js b/loopback/common/methods/application/executeProc.js index 5859611d98..a8825da0fc 100644 --- a/loopback/common/methods/application/executeProc.js +++ b/loopback/common/methods/application/executeProc.js @@ -1,7 +1,7 @@ module.exports = Self => { Self.remoteMethodCtx('executeProc', { description: 'Return result of procedure', - accessType: '*', + accessType: 'EXECUTE', accepts: [ { arg: 'routine',