Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encrypting password #74

Open
somtochiama opened this issue Mar 8, 2019 · 3 comments
Open

Encrypting password #74

somtochiama opened this issue Mar 8, 2019 · 3 comments
Labels

Comments

@somtochiama
Copy link
Contributor

somtochiama commented Mar 8, 2019

The passwords for the admins are stored without being encrypted. It would be more secure if they were encrypted with an npm package like bcrypt-nodejs.

I am ready to work on this issue if it is valid enough

@NikhilM98
Copy link
Collaborator

NikhilM98 commented Mar 8, 2019

I agree that storing plain text password in the DB is a major security risk. In the case of NoSQL injection attack on the school server, the attacker can retrieve plain text passwords of the students and the admin.

Admin and students are created using a common route (/auth/signup). Roles are assigned to them. The function for sign-up:

exports.addUser = function(req, res) {
//validate
if (!req.body.user) {
res.status(401).send({
'error': 'User object not defined!',
'code': 21
});
return;
}
//parse user details
var user = JSON.parse(req.body.user);
//add timestamp & language
user.created_time = +new Date();
user.timestamp = +new Date();
user.role = (user.role ? user.role.toLowerCase() : 'student');
//validation for fields [password, name]
if (!user.password || !user.name) {
res.status(401).send({
'error': "Invalid user object!",
'code': 2
});
}
// validate on the basis of user's role for logged in case
if (req.user) {
if (req.user.role == 'student') {
return res.status(401).send({
'error': 'You don\'t have permission to perform this action',
'code': 19
});
}
}

Small children are not expected to remember their passwords, so I think it's good to let the admin retrieve plain text password.

If you try to hash admin password, then you've to accordingly modify authentication. We then no longer be able to use the same functions in admin and student authentication.

Instead of using a hashing algorithm like bcrypt we can use encryption on student and admin passwords. When the admin wants to view student password, the admin can use that encryption key to decrypt the password. (Using common authentication functions)

@llaske @tarunsinghal92 would have a better idea that if we need to use encryption or not.

Edit: I think there is a flaw in my proposal. If the attacker has access to the DB then he can retrieve admin token. Login as admin using the token and view student password.

@somtochiama
Copy link
Contributor Author

Okay, you make a valid point. I was thinking that we would just check the the role of the user first before encrypting it but which ever way that works. I will wait for further input from others

@llaske
Copy link
Owner

llaske commented Mar 10, 2019

We need to keep the way to retrieve students password as simple as possible. There is nothing like an e-mail address (neither for students, neither for admin) where we could send a password or a link to reset the password. I don't want to force young children to create an e-mail and I don't want to depend of an external e-mail platforms.
So I'm not in favor to add something that could risk to lock accounts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants