El problema es la forma en que manejas la consulta a la base de datos, además podrías mejorar las cosas un poco en tu modelo, para evitar duplicidad de datos. También te sugiero que no uses SHA1 para realizar el hashing de las contraseñas, ya que no es tan seguro como debería.
¿Encriptado o Hashing?
Debo aclararte que existe una diferencia entre encriptado y hashing. En la primera, podemos realizar la operación en ambas vías, es decir, tomar un texto plano y encriptar su contenido (codificarlo) para que no sea legible, solamente quien tenga las llaves de desencriptado adecuadas podrá leer el contenido del mismo. Por lo tanto, encriptar datos tiene como finalidad transmitirlos de manera segura entre un emisor y un receptor (quienes conocen las llaves de encriptado y desencriptado), de tal manera que el texto original puede ser encriptado para ser ilegible y luego puede ser desencriptado para ser nuevamente legible.
Un hashing por el contrario funciona en una sola vía, es decir, toma un texto plano y lo convierte en una serie de caracteres y símbolos de tal manera que no se puede volver desde dicho estado al texto original. (O al menos esa es la idea).
SHA1 vs. SHA2
Dado que usas la librería de encriptado nativa de Node: Crypto, mi recomendación es que uses un algoritmo diferente al que usas actualmente para realizar el hashing de la contraseña.
SHA1 a mostrado ser vulnerable a medida que pasa el tiempo. La posibilidad de colisiones y el auge de las tablas Rainbow, han hecho que los sistemas de seguridad se muevan hacia algoritmos un poco más costosos como SHA2 o Blowfish. Dado que Crypto da soporte para el algoritmo SHA512, lo ideal es usar dicho algoritmo para realizar el hashing.
PROBLEMA
Se desea realizar una actualización a un documento almacenado en una base de datos MongoDB, usando Node, Express y Mongoose. La actualización se realizará mediante la búsqueda del documento en la base de datos usando como consulta un campo de dicho documento.
SOLUCIÓN
Lo primero que debemos hacer es cambiar algunas cosas en nuestro modelo, ya que tal como está es susceptible a errores, en particular a errores de datos duplicados. Por otra parte, vamos a realizar una pequeña refactorización en nuestro programa, con esto lo que se pretende es mejorar el código y tener un modelo aún más reusable y modularizado.
MODELO USER
Aunque no existe un manual estricto sobre ¿Cómo crear modelos con Mongoose?, si que existen los Principios S.O.L.I.D que nos dan una pauta para nuestros programas en general.
En el modelo de usuario que actualmente usas puedo notar algunos detalles que se pueden (y se deben) mejorar.
Identificacion => identificacion
En primer lugar tenemos el campo que llamas Identificacion
. Vamos a cambiar el nombre de dicho campo por el mismo pero todo en minúsculas (lower Camel Case). Es por convención, puedo entender que dicho campo revista alguna importancia, pero el poner la primera letra mayúscula no implica que le damos más importancia. Puedes leer un poco sobre convención de nombres (programación) como una orientación sobre esto.
Otra cosa que haremos con dicho campo es configurarlo como unique
. Si mal no entiendo tu modelo, pretendes realizar búsquedas utilizando el campo identificacion
como parámetro de consulta. Por lo tanto estoy asumiendo que dicho campo ha de ser único para el documento.
Normalmente se utiliza el campo _id
que existe en cada documento de MongoDB, pero a veces hay situaciones en las cuales se requiere utilizar otro campo diferente, también como campo único.
Si tu modelo no requiere usar un valor tipo ObjectId
de MongoDB, puedes reescribir el campo _id
de MongoDB usando tu propio valor. Sin embargo, debes asegurar que dicho valor sea único por documento.
encriptar() => hashPassword()
Como bien he dicho anteriormente, el proceso que se realiza con la contraseña de usuario se llama hashing. Tal vez una de las fuentes de confusión es el hecho de que la librería para realizar el proceso se llama Crypto. Por lo tanto, será conveniente llamar al método con un nombre más adecuado, ya que así nuestro método deja claro a otros programadores la tarea que realiza.
Un punto importante que voy a destacar es el hecho de poder incorporar el método en nuestro modelo, en vez de ser un método solitario que debemos escribir o importar desde algún controlador.
Si cada vez que debemos autenticar un usuario en un controlador necesitamos escribir o importar la función de hashing, entonces estamos siendo repetitivos. Por lo tanto, siendo la contraseña una propiedad de nuestro modelo, el hashing puede (y debe) ser un método de nuestro modelo.
Español => English
Este punto puede traer roncha. Hay quienes prefieren usar su idioma nativo para todo en su código, sin embargo esto puede llegar a ser contraproducente. En particular, yo mismo he escrito código con variables en español. Sin embargo, casi todos los frameworks y librerías vienen en inglés. Los métodos normalmente son palabras del habla inglesa. send()
, set()
, get()
, son algunos ejemplos.
Digo esto, porque en particular Mongoose, tomará el nombre de nuestro modelo y creará una colección usando el plural (en inglés) del nombre de nuestro modelo. Por lo tanto, es posible que las colecciones en MongoDB tengan nombres un poco extraños si nuestros modelos tienen nombres en español.
Sin embargo, esto es elección propia. Mi sugerencia es y será que usemos el idioma inglés al programar, como una buena práctica.
Virtuals, Methods y Hooks
Para poder incluir en nuestro Modelo el método de hashing, vamos a hacer uso de algunas características de Mongoose.
Mongoose nos permite crear modelos a partir de un Schema.
Un Schema es básicamente un objeto que define cómo son los documentos dentro de una colección en MongoDB. Podemos decir que un Schema define un patrón de documentos.
Un Modelo en cambio, es un constructor compilado a partir de un Schema, y es lo que nos va a permitir realizar operaciones CRUD con un documento de MongoDB.
En Mongoose, los Modelos tienen métodos propios (find()
, save()
, etc.), pero también podemos crear nuestros propios métodos.
Crearemos 2 métodos: hashPassword()
y isAuthenticated()
, ambos métodos son totalmente inherentes a nuestro Modelo, ya que realizan operaciones sobre el mismo. Dichos métodos serán parte del atributo methods
de nuestro Schema.
hashPassword
hashPasword: function(password) {
if(!password) {
throw new Error('No password supplied for hash!');
}
const hash = crypto.createHmac('sha512', 'My Secret String').update(password).digest('hex');
return hash;
}),
isAuthenticated
isAuthenticated: function(password) {
if(!password) {
throw new Error('No password supplied for authenticate!');
}
const hash = this.hashPassword(password);
return hash === this.hashed_password ? true : false;
}),
En el primer método expuesto, generamos un valor hash usando como entrada la contraseña tecleada por el usuario.
En el segundo método comparamos el valor almacenado en el campo hashed_password
de nuestro documento con el valor generado a partir de la contraseña ingresada por el usuario. Si ambos coinciden devolvemos true
y en caso contrario devolvemos false
.
Entonces, ya tenemos un método para generar el hash
de nuestra contraseña y un método para verificar si la contraseña introducida es válida.
Para manejar el valor del campo hashed_password
y volver a generar un nuevo valor en caso que el usuario actualice su contraseña, debemos crear un campo virtual y además realizaremos validaciones previas al método save()
del documento.
Primero crearemos nuestro campo virtual. Un campo virtual es aquel campo que permite almacenar y obtener datos de un documento, pero que no será persistido (grabado) en nuestra base de datos. Nuestro campo virtual se llamará password
, así tenemos un valor que podemos pedir o solicitar del lado cliente y el campo hashed_password
será un campo que solo manejaremos del lado servidor.
Campo virtual 'password'
UserSchema.virtual('password')
.set(function(password) {
this._pasword = password;
this.hashed_password = 'Password has been modified, need to re-hash.';
})
.get(function() {
return this._password;
});
En el código anterior hemos creado un campo virtual llamado password
, el cual tiene dos métodos.
El método set()
, almacena el password que viene en texto plano en una variable temporal llamada _password
, y altera el valor del campo hashed_password
de nuestro documento. De esta forma podremos saber si el usuario ha cambiado el password al hacer un update
a su perfil.
El método get()
por su parte, devuelve el valor almacenado en la variable temporal, de esta forma luego podremos realizar el hash
sobre este valor en caso de cambio o cuando se crea un nuevo documento.
Ahora solo queda añadir una validación sobre el campo hashed_password
. Normalmente el tipo de validación variará de acuerdo a nuestro diseño, pero por norma general debemos pedir un mínimo de caracteres para el campo de contraseña.
Para realizar la validación vamos a usar un método llamado validate()
de la clase Schema de mongoose. Básicamente lo que hace este método es realizar las operaciones que le pasemos dentro de una función de validación que se le pasa como argumento. Dado que nuestro campo hashed_password
depende de nuestro campo virtual password
debemos añadir la validación usando el método path()
. Además, la validación generará un error sobre el campo virtual password
.
Validación sobre hashed_password
UserSchema.path('hashed_password').validate(function() {
if(this._password && this._password.length < 6) {
this.invalidate('password', 'Password must have at least 6 characters');
}
});
Con esto ya tenemos una forma de validar el campo de contraseña. He escogido 6 caracteres como valor mínimo, esto lo puedes hacer según el requerimiento. Existen validaciones más elaboradas usando expresiones regulares.
Por último, crearemos un hook
, que simplemente es una función middleware que se ejecutará en un momento dado durante la ejecución de alguna operación sobre nuestro modelo.
Nuestro hook
será un middleware tipo pre, y su finalidad será validar si un documento tipo usuario es nuevo o si se se está alterando el valor de hashed_password
en un documento existente.
¿Porqué debemos validar esto? En principio, cada vez que se crea un nuevo usuario, el campo hashed_password
está vacío, por lo tanto debemos generar el hash antes de almacenarlo en la DB. En cambio, si el usuario no es nuevo, quiere decir que el proceso que estamos realizando sobre el documento es de actualización. Si ese es el caso, debemos verificar si se ha alterado el campo hashed_password
(recordemos que esto solo sucede si se ha llamado al método set
de nuestro campo virtual), si el campo ha sido alterado, se debe llamar al proceso de hashing para almacenar el nuevo valor en nuestro campo hashed_password
. Una vez actualizado el nuevo hash se puede continuar el proceso de actualización del documento.
Pre 'validate' middleware
UserSchema.pre('validate', function(next) {
const user = this;
if(!user.isNew && !user.isModified('hashed_password')) {
next();
} else {
const password = user.password;
user.hashed_password = user.hashPassword(password);
next();
}
});
Con esto ya tenemos listas las modificaciones a nuestro modelo User
, y hemos incorporado dos métodos fundamentales a nuestro modelo. Ya no tendremos que escribir una rutina de autenticación o de hashing nunca más, ya que las mismas vienen incorporadas en nuestro modelo.
Nuestro archivo del modelo podría verse mas o menos así:
user.model.js
const mongoose = require('mongoose');
const crypto = require('crypto');
const UserSchema= new mongoose.Schema({
identificacion: {
type:String,
min:7,
max:10,
required: 'Identificación es requerida',
unique: 'Identificación ya existe'
},
nombre: {
type: String,
maxlength: [50, 'Nombre muy largo'],
minlength: [3,'Nombre muy corto'],
required: true
},
apellido: {
type: String,
max: 50,
required: true
},
correo: {
type: String,
max: 80,
required: 'El correo es requerido'
},
telefono: {
type: String,
max: 10,
required: true
},
hashed_password: {
type:String,
required: 'Password es requerido'
}
});
UserSchema.methods = {
hashPasword: function(password) {
if(!password) {
throw new Error('No password supplied for hash!');
}
const hash = crypto.createHmac('sha512', 'My Secret String').update(password).digest('hex');
return hash;
}),
isAuthenticated: function(password) {
if(!password) {
throw new Error('No password supplied for authenticate!');
}
const hash = this.hashPassword(password);
return hash === this.hashed_password ? true : false;
})
}
UserSchema.virtual('password')
.set(function(password) {
this._pasword = password;
this.hashed_password = 'Password has been modified, need to re-hash.';
})
.get(function() {
return this._password;
});
UserSchema.path('hashed_password').validate(function() {
if(this._password && this._password.length < 6) {
this.invalidate('password', 'Password must have at least 6 characters');
}
});
UserSchema.pre('validate', function(next) {
const user = this;
if(!user.isNew && !user.isModified('hashed_password')) {
next();
} else {
const password = user.password;
user.hashed_password = user.hashPassword(password);
next();
}
});
module.exports = mongoose.model('User', UserSchema);
CONTROLADOR USER
Ya tenemos un modelo totalmente equipado con el que podemos trabajar, ahora modificaremos algunas cosas en nuestro controlador de Usuario.
Empezaremos con el método signup()
, y aunque trataré de no modificar mucho la lógica de tu aplicación, debo refactorizar para poder hacer uso de nuestro nuevo modelo.
Método Signup
En tu controlador, tal como lo tienes actualmente es bastante repetitivo. Si estamos creando un nuevo usuario, tal vez no es necesario ir capturando los valores de los diferentes campos del modelo uno a uno.
Normalmente una solicitud POST del tipo que estás realizando incluye los datos dentro del body
de la solicitud.
Si las cosas las haces correctamente, en el body
de tu solicitud solo vienen los valores necesarios para crear un nuevo usuario. Por lo tanto podemos hacer lo siguiente:
let user = new User(req.body);
Con esta simple línea hemos creado un nuevo objeto tipo User con todos los valores que vienen desde el cliente en el body
de la solicitud.
Como nuestro Modelo tiene incorporado un método para hacer el hashing no necesitamos hacerlo en nuestro controlador.
Otra cosa que realizas en el controlador es verificar si el usuario existe. Esto, aunque suene lógico, realmente sobrecarga el método con una llamada adicional a la base de datos: una vez para buscar si el usuario existe y la segunda vez para salvar el nuevo usuario.
Dado que le hemos dado al campo identificacion
el atributo de unique
, sólo necesitamos 1 llamada a la base de datos para salvar el usuario, si el usuario no existe se creará un nuevo documento en la colección, en cambio si el usuario ya existe, la base de datos nos devolverá un error indicándonos que el documento ya existe en la colección.
En todo caso, si deseamos hacer una comprobación antes de realizar el proceso de creación del documento, la misma será sobre el objeto body
de nuestra solicitud.
Dicho esto nuestro método signup puede quedar de la siguiente forma:
signup
const User = require('path/to/user.model');
exports.signUp_post = (req, res, next) => {
if(!req.body) {
return res.status(400).json({
error: 'No se recibieron datos para procesar'
});
}
let user = new User(req.body);
user.save((err, usr) => {
if(err) {
console.log('Error: ', err.message);
return res.status(500).json({
error: err.message
});
}
console.log('Usuario creado con _id: ', usr._id);
return res.status(200).render('signUp', { msg1: 'El usuario ha sido creado'});
});
}
Con esto ya tenemos nuestro método signup usando unas pocas líneas de código.
Si revisas el código puedes ver que no hago uso de la palabra clave var
para declarar variables. NodeJS soporta desde su versión 6.4 la nomenclatura de ES5 que incluye a let
y const
. Es recomendable el uso de let
y const
y evitar el uso de var
. Puedes ver esta pregunta en S.O. y la excelente respuesta dada por @PabloLozano en: var, let, const… o nada en Javascript.
Método update_user
Por último, vamos a realizar los cambios necesarios en tu método de actualización, para adaptarlo a nuestro nuevo Modelo.
Si usaras el campo _id
de tu documento, podríamos hacer uso del método findById()
de Mongoose. Sin embargo, ese no es el caso.
Del código que has posteado, puedo asumir que deseas buscar el documento mediante el campo identificación
. Y como dicho campo lo hemos declarado como unique
en nuestro Modelo, la búsqueda debería arrojar como máximo un solo resultado (digo máximo, porque puede arrojar ningún resultado si el usuario no existe).
En tu código, capturas el valor de identificación como un parámetro de la solicitud, pero también haces algo bastante peculiar:
var user = req.params.Identificacion;
var Identificación = req.body.Identificacion;
En estas dos líneas de código parece que los campos Identificacion
que vienen como parámetro de la solicitud y el que viene en el cuerpo de la solicitud son diferentes.
Dado que puedes actualizar el campo identificacion
de tu documento, no es necesario extraerlo en una variable aparte desde tu body
.
La lógica del proceso de actualización es algo parecida a esto:
- Realizar el SignIn para acceder a la información de usuario y de esta forma se obtiene el valor de
identificación
actual del usuario
- Llenar el formulario con los nuevos datos de usuario (puede incluir un nuevo campo
identificacion
)
- Enviar la consulta al servidor (normalmente usando el método PUT de HTTP), el valor de identificación (actual) se envía como un parámetro de la solicitud (
/ruta/usuario/:identificacion
)
- Realizar una búsqueda en la base de datos usando el parámetro de identificación y si encontramos el documento lo actualizamos con los datos que fueron enviados en el
body
de la solicitud.
- Informar el resultado de la actualización.
Para poder llevar a cabo la tarea de actualización usaremos el método findOneAndUpdate()
de Mongoose.
En tu controlador, tenías el siguiente valor como query
:
{user:user}
Sin embargo, user
no es un campo que exista en tu documento, por lo cual el método con seguridad no realizaba la tarea que crees que debía realizar. El documento query
que debes pasar al método es:
{identificacion: identificacion}
Ya tenemos toda la información necesaria para escribir nuestro método update_user
.
update_user
const User = require('path/to/user.model');
exports.update_user_post = (req, res, next) => {
const identificacion = req.params.identificacion; // <= IMPORTANTE: el campo se llama identificacion con minúscula, en el lado cliente
User.findOneAndUpdate({identificacion: identificacion}, req.body, {new: true})
.exec((err, result) => {
if(err) {
console.log(err.message);
return res.status(500).json({
error: err.message
});
}
if(!result) {
console.log('No se encontraron datos para identificacion: ', identificacion);
return res.status(400).json({
message: 'No se encuentra el usuario con identificacion: ' + identificacion
});
}
console.log(result); // <= Aqui vemos el documento actualizado
return res.status(200).render('update_user', {message2: 'El usuario ha sido actualizado'});
});
}
Con esto tus controladores deberían funcionar correctamente.
Nótese que al método findOneAndUpdate()
le he pasado 3 parámetros:
{identificacion: identificacion}
: esta es la query de búsqueda, debe arrojar 0 o 1 resultado. Si el campo identificacion es de tipo
unique` entonces solo debe existir un documento por identificación.
req.body
: este es el objeto que contiene los campos a actualizar en el documento.
{new: true}
: este parámetro le dice a Mongoose que al realizar la actualización el resultado devuelto sea el documento actualizado y no el documento encontrado en la búsqueda antes de la actualización.
Espero que esto aclare tus dudas y te permita mejorar tu modelo y los mñetodo que usas para trabajar con Mongoose, Express y NodeJS.