Open somtochiama opened 5 years ago
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:
https://github.com/llaske/sugarizer-server/blob/be03d5e7a79f5571e56e1bd31acaf7882a1eacef/api/controller/users.js#L358-L394
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.
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
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.
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