6067-vnUser_privileges_and_verifyEmail #1764

Merged
alexm merged 15 commits from 6067-vnUser_privileges_and_verifyEmail into dev 2023-10-19 12:13:20 +00:00
10 changed files with 120 additions and 73 deletions
Showing only changes of commit 6cf1095b10 - Show all commits

View File

@ -0,0 +1,41 @@
module.exports = Self => {
Self.remoteMethodCtx('updateUser', {
description: 'Update user data',
accepts: [
{
arg: 'id',
type: 'integer',
description: 'The user id',
required: true,
http: {source: 'path'}
}, {
arg: 'name',
type: 'string',
description: 'The user name',
}, {
arg: 'nickname',
type: 'string',
description: 'The user nickname',
}, {
arg: 'email',
type: 'string',
description: 'The user email'
}, {
arg: 'lang',
type: 'string',
description: 'The user lang'
}
],
http: {
path: `/:id/update-user`,
verb: 'PATCH'
}
});
Self.updateUser = async(ctx, id) => {
await Self.userSecurity(ctx, id);
const user = await Self.app.models.VnUser.findById(id,
{fields: ['id', 'name', 'nickname', 'email', 'lang', 'password']});
alexm marked this conversation as resolved
Review

En lugar de hacer findById y luego updateAttributes, utilizar upsert.

En lugar de hacer `findById` y luego `updateAttributes`, utilizar `upsert`.
await user.updateAttributes(ctx.args);
alexm marked this conversation as resolved
Review

En lugar de utilizar ctx.args listar parámetros en la definición de la función y pasarlos todos excepto el id.

En lugar de utilizar `ctx.args` listar parámetros en la definición de la función y pasarlos todos excepto el `id`.
};
};

View File

@ -12,4 +12,42 @@ describe('loopback model VnUser', () => {
expect(result).toBeFalsy(); expect(result).toBeFalsy();
}); });
describe('userSecurity', () => {
const itManagementId = 115;
const hrId = 37;
const employeeId = 1;
it('should check if you are the same user', async() => {
const ctx = {options: {accessToken: {userId: employeeId}}};
await models.VnUser.userSecurity(ctx, employeeId);
});
it('should check for higher privileges', async() => {
const ctx = {options: {accessToken: {userId: itManagementId}}};
await models.VnUser.userSecurity(ctx, employeeId);
});
it('should check if you have medium privileges and the user email is not verified', async() => {
const ctx = {options: {accessToken: {userId: hrId}}};
await models.VnUser.userSecurity(ctx, employeeId);
});
it('should throw an error if you have medium privileges and the users email is verified', async() => {
const tx = await models.VnUser.beginTransaction({});
const ctx = {options: {accessToken: {userId: hrId}}};
try {
const options = {transaction: tx};
const userToUpdate = await models.VnUser.findById(1, null, options);
userToUpdate.updateAttribute('emailVerified', 1, options);
await models.VnUser.userSecurity(ctx, employeeId, options);
await tx.rollback();
} catch (error) {
await tx.rollback();
expect(error.message).toEqual(`You don't have enough privileges`);
}
});
});
}); });

View File

@ -13,6 +13,7 @@ module.exports = function(Self) {
require('../methods/vn-user/privileges')(Self); require('../methods/vn-user/privileges')(Self);
require('../methods/vn-user/validate-auth')(Self); require('../methods/vn-user/validate-auth')(Self);
require('../methods/vn-user/renew-token')(Self); require('../methods/vn-user/renew-token')(Self);
require('../methods/vn-user/update-user')(Self);
Self.definition.settings.acls = Self.definition.settings.acls.filter(acl => acl.property !== 'create'); Self.definition.settings.acls = Self.definition.settings.acls.filter(acl => acl.property !== 'create');
@ -179,32 +180,31 @@ module.exports = function(Self) {
Self.sharedClass._methods.find(method => method.name == 'changePassword').ctor.settings.acls Self.sharedClass._methods.find(method => method.name == 'changePassword').ctor.settings.acls
.filter(acl => acl.property != 'changePassword'); .filter(acl => acl.property != 'changePassword');
Self.observe('before save', async ctx => { Self.userSecurity = async(ctx, userId, options) => {
const instance = ctx.currentInstance || ctx.instance;
console.log(ctx);
await Self.userSecurity(ctx, instance.id);
});
Self.userSecurity = async(ctx, userId) => {
const models = Self.app.models; const models = Self.app.models;
const accessToken = ctx.options.accessToken || LoopBackContext.getCurrentContext().active.accessToken; const accessToken = ctx?.options?.accessToken || LoopBackContext.getCurrentContext().active.accessToken;
console.log(accessToken, LoopBackContext.getCurrentContext().active.http.req);
const ctxToken = {req: {accessToken}}; const ctxToken = {req: {accessToken}};
const hasHigherPrivileges = await models.ACL.checkAccessAcl(ctxToken, 'VnUser', 'higherPrivileges'); if (userId === accessToken.userId) return;
const myOptions = {};
if (typeof options == 'object')
Object.assign(myOptions, options);
const hasHigherPrivileges = await models.ACL.checkAccessAcl(ctxToken, 'VnUser', 'higherPrivileges', myOptions);
if (hasHigherPrivileges) return; if (hasHigherPrivileges) return;
const hasMediumPrivileges = await models.ACL.checkAccessAcl(ctxToken, 'VnUser', 'mediumPrivileges'); const hasMediumPrivileges = await models.ACL.checkAccessAcl(ctxToken, 'VnUser', 'mediumPrivileges', myOptions);
const user = await models.VnUser.findById(userId, {fields: ['id', 'emailVerified']}); const user = await models.VnUser.findById(userId, {fields: ['id', 'emailVerified']}, myOptions);
if (!user.emailVerified && hasMediumPrivileges) return; if (!user.emailVerified && hasMediumPrivileges) return;
if (userId != accessToken.userId)
throw new UserError(`You don't have enough privileges`); throw new UserError(`You don't have enough privileges`);
}; };
// FIXME: https://redmine.verdnatura.es/issues/5761 Self.observe('after save', async ctx => {
Self.afterRemote('prototype.patchAttributes', async(ctx, instance) => { const newEmail = ctx?.instance?.email;
if (!ctx.args || !ctx.args.data.email) return; const oldEmail = ctx?.hookState?.oldInstance?.email;
if (!ctx.isNewInstance && (!newEmail || !oldEmail || newEmail == oldEmail)) return;
alexm marked this conversation as resolved
Review

Sobre isNewInstance.
Mirar la nota https://redmine.verdnatura.es/issues/5761#note-8

Sobre isNewInstance. Mirar la nota https://redmine.verdnatura.es/issues/5761#note-8
const loopBackContext = LoopBackContext.getCurrentContext(); const loopBackContext = LoopBackContext.getCurrentContext();
const httpCtx = {req: loopBackContext.active}; const httpCtx = {req: loopBackContext.active};
@ -217,8 +217,7 @@ module.exports = function(Self) {
async send(verifyOptions, cb) { async send(verifyOptions, cb) {
const params = { const params = {
url: verifyOptions.verifyHref, url: verifyOptions.verifyHref,
recipient: verifyOptions.to, recipient: verifyOptions.to
lang: ctx.req.getLocale()
}; };
const email = new Email('email-verify', params); const email = new Email('email-verify', params);
@ -230,9 +229,9 @@ module.exports = function(Self) {
const options = { const options = {
type: 'email', type: 'email',
to: instance.email, to: newEmail,
from: {}, from: {},
redirect: `${origin}/#!/account/${instance.id}/basic-data?emailConfirmed`, redirect: `${origin}/#!/account/${ctx.instance.id}/basic-data?emailConfirmed`,
template: false, template: false,
mailer: new Mailer, mailer: new Mailer,
host: url[1].split('/')[2], host: url[1].split('/')[2],
@ -241,6 +240,6 @@ module.exports = function(Self) {
user: Self user: Self
}; };
await instance.verify(options); await ctx.instance.verify(options, ctx.options);
}); });
}; };

View File

@ -13,11 +13,12 @@
"type": "number", "type": "number",
"id": true "id": true
}, },
"username": { "name": {
"type": "string", "type": "string",
"mysql": { "required": true
"columnName": "name" },
} "username": {
"type": "string"
}, },
"password": { "password": {
"type": "string", "type": "string",
@ -123,12 +124,6 @@
"principalType": "ROLE", "principalType": "ROLE",
"principalId": "$authenticated", "principalId": "$authenticated",
"permission": "ALLOW" "permission": "ALLOW"
},
{
"principalType": "ROLE",
"principalId": "$authenticated",
"permission": "ALLOW",
"property": "patchAttributes"
} }
], ],
"scopes": { "scopes": {

View File

@ -1,12 +1,7 @@
DELETE FROM `salix`.`ACL`
WHERE
model = 'MailForward'
AND accessType = '*'
AND property = '*'
AND principalId = 'hr';
INSERT INTO `salix`.`ACL` (model, property, accessType, permission, principalType, principalId) INSERT INTO `salix`.`ACL` (model, property, accessType, permission, principalType, principalId)
VALUES VALUES
('VnUser', 'higherPrivileges', '*', 'ALLOW', 'ROLE', 'itManagement'), ('VnUser', 'higherPrivileges', '*', 'ALLOW', 'ROLE', 'itManagement'),
('VnUser', 'mediumPrivileges', '*', 'ALLOW', 'ROLE', 'hr'); ('VnUser', 'mediumPrivileges', '*', 'ALLOW', 'ROLE', 'hr'),
('VnUser', 'updateUser', '*', 'ALLOW', 'ROLE', 'employee');
ALTER TABLE `account`.`user` ADD `username` varchar(30) AS (name) VIRTUAL;

View File

@ -1,9 +1,5 @@
module.exports = Self => { module.exports = Self => {
Self.observe('loaded', async ctx => {
if (!ctx.data.account) return;
await Self.app.models.VnUser.userSecurity(ctx, ctx.data.account);
});
Self.observe('before save', async ctx => { Self.observe('before save', async ctx => {
const instance = ctx.currentInstance || ctx.instance; const instance = ctx.currentInstance || ctx.instance;
await Self.app.models.VnUser.userSecurity(ctx, instance.account); await Self.app.models.VnUser.userSecurity(ctx, instance.account);

View File

@ -21,19 +21,5 @@
"model": "VnUser", "model": "VnUser",
"foreignKey": "account" "foreignKey": "account"
} }
},
"acls": [
{
"accessType": "READ",
"principalType": "ROLE",
"principalId": "$authenticated",
"permission": "ALLOW"
},
{
"accessType": "WRITE",
"principalType": "ROLE",
"principalId": "$authenticated",
"permission": "ALLOW"
} }
]
} }

View File

@ -1,11 +1,9 @@
<mg-ajax path="VnUsers/{{post.params.id}}" options="vnPost"></mg-ajax> <mg-ajax path="VnUsers/{{patch.params.id}}/update-user" options="vnPatch"></mg-ajax>
<vn-watcher <vn-watcher
vn-id="watcher" vn-id="watcher"
url="VnUsers/preview"
data="$ctrl.user" data="$ctrl.user"
where="{id: $ctrl.$params.id}"
form="form" form="form"
save="post"> save="patch">
</vn-watcher> </vn-watcher>
<form <form
name="form" name="form"

View File

@ -2,13 +2,6 @@ import ngModule from '../module';
import Section from 'salix/components/section'; import Section from 'salix/components/section';
export default class Controller extends Section { export default class Controller extends Section {
set user(value) {
this._user = value;
console.log(value);
}
get user() {
return this._user;
}
$onInit() { $onInit() {
if (this.$params.emailConfirmed) if (this.$params.emailConfirmed)
this.vnApp.showSuccess(this.$t('Email verified successfully!')); this.vnApp.showSuccess(this.$t('Email verified successfully!'));
@ -25,5 +18,8 @@ ngModule.component('vnUserBasicData', {
controller: Controller, controller: Controller,
require: { require: {
card: '^vnUserCard' card: '^vnUserCard'
},
bindings: {
user: '<'
} }
}); });

View File

@ -77,7 +77,10 @@
"url": "/basic-data?emailConfirmed", "url": "/basic-data?emailConfirmed",
"state": "account.card.basicData", "state": "account.card.basicData",
"component": "vn-user-basic-data", "component": "vn-user-basic-data",
"description": "Basic data" "description": "Basic data",
"params": {
"user": "$ctrl.user"
}
}, },
{ {
"url" : "/log", "url" : "/log",