verdnatura/salix
verdnatura
/
salix
3
4
Fork 0
Code Issues Pull Requests 59 Releases Wiki Activity

#6744 fix worker setPassword #2027

Merged
jorgep merged 18 commits from 6744-fixWorkerSetPassword into dev 2024-03-15 09:48:14 +00:00
Conversation 0 Commits 18 Files Changed 8 +4 -4
4 changed files with 4 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3bf25e5759 - Show all commits

2
loopback/locale/en.json
View File

@ -210,5 +210,5 @@
"CountryFK cannot be empty": "Country cannot be empty",
"You are not allowed to modify the alias": "You are not allowed to modify the alias",
"You already have the mailAlias": "You already have the mailAlias",
"The email has been already verified": "The email has been already verified"
"This password can only be changed by the user themselves": "This password can only be changed by the user themselves"
}

2
loopback/locale/es.json
View File

@ -345,5 +345,5 @@
"Cmr file does not exist": "El archivo del cmr no existe",
"You are not allowed to modify the alias": "No estás autorizado a modificar el alias",
"No tickets to invoice": "No hay tickets para facturar",
"The email has been already verified": "El correo ya ha sido verificado"
"This password can only be changed by the user themselves": "Esta contraseña solo puede ser modificada por el propio usuario"
}

2
modules/account/back/models/account.js
View File

@ -13,7 +13,7 @@ module.exports = Self => {
Self.setUnverifiedPassword = async(id, pass, options) => {
const user = await models.VnUser.findById(id, null, options);
if (user.emailVerified) throw new ForbiddenError('The email has been already verified');
if (user.emailVerified) throw new ForbiddenError('This password can only be changed by the user themselves');
jgallego commented 2024-02-23 06:20:42 +00:00
Outdated
Review
Copy Link

Si alguien intenta cambiar un password y le mostramos este mensaje de error, no va a saber relacionarlo..
Pensando en Norman yo mostraria algo así como "Esta contraseña solo puede ser modificada por el propio usuario"

Y asegurate que VnUser.setPassword tiene los permisos correctos y nadie lo puede llamar.

Si alguien intenta cambiar un password y le mostramos este mensaje de error, no va a saber relacionarlo.. Pensando en Norman yo mostraria algo así como "Esta contraseña solo puede ser modificada por el propio usuario" Y asegurate que VnUser.setPassword tiene los permisos correctos y nadie lo puede llamar.
jorgep commented 2024-02-23 08:27:56 +00:00
Outdated
Review
Copy Link

En principio lo reviso con isSubordinate en worker->setPassword y dentro de account->setUnverifiedPassword compruebo si el email está verificado. Hay que cambiar el enfoque? @juan @jgallego . setUnverifiedPassword no se puede llamar desde el exterior(http request), solo internamente.

En principio lo reviso con isSubordinate en worker->setPassword y dentro de account->setUnverifiedPassword compruebo si el email está verificado. Hay que cambiar el enfoque? @juan @jgallego . setUnverifiedPassword no se puede llamar desde el exterior(http request), solo internamente.
jorgep commented 2024-02-23 08:35:55 +00:00
Outdated
Review
Copy Link

Traducción cambiada

Traducción cambiada
await models.VnUser.setPassword(id, pass, options);
await user.updateAttribute('emailVerified', true, options);
jorgep marked this conversation as resolved
jgallego commented 2024-02-23 11:24:43 +00:00
Review
Copy Link

porque activas el correo aqui? si el usuario no consigue entrar por cualquier razón no debería verificarse el mail

porque activas el correo aqui? si el usuario no consigue entrar por cualquier razón no debería verificarse el mail
jorgep commented 2024-02-26 07:22:27 +00:00
Review
Copy Link

me lo pidio expresamente @juan . A que te refieres con que no consigue entrar?

me lo pidio expresamente @juan . A que te refieres con que no consigue entrar?
jgallego commented 2024-02-26 07:58:41 +00:00
Review
Copy Link

Considero que és un mal enfoque.
El hecho de cambiar la contraseña no es lo mismo que hacer un login satisfactorio.
Bajo mi punto de vista poner el email verificado a true sólo se debería hacer cuando el usuario ha conseguido hacer un login satisfactorio

Considero que és un mal enfoque. El hecho de cambiar la contraseña no es lo mismo que hacer un login satisfactorio. Bajo mi punto de vista poner el email verificado a true sólo se debería hacer cuando el usuario ha conseguido hacer un login satisfactorio
jorgep commented 2024-02-26 08:52:29 +00:00
Review
Copy Link

Paso la tarea a feedback pues

Paso la tarea a feedback pues
jgallego commented 2024-02-28 08:15:59 +00:00
Review
Copy Link

después de hablar con Juan quitar await user.updateAttribute('emailVerified', true, options);
poner el email verificado a true sólo se debería hacer cuando el usuario ha conseguido hacer un login satisfactorio

después de hablar con Juan quitar await user.updateAttribute('emailVerified', true, options); poner el email verificado a true sólo se debería hacer cuando el usuario ha conseguido hacer un login satisfactorio

2
modules/worker/back/methods/worker/specs/setPassword.spec.js
View File

@ -42,7 +42,7 @@ describe('worker setPassword()', () => {
await tx.rollback();
} catch (e) {
expect(e.message).toEqual(`The email has been already verified`);
expect(e.message).toEqual(`This password can only be changed by the user themselves`);
await tx.rollback();
}
});