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

[frontend] Adding the use of Argon2id for password hashing #978

Merged
merged 14 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion inginious/frontend/pages/course_admin/danger_zone.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def get_backup_list(self, course):

def page(self, course, msg="", error=False):
""" Get all data and display the page """
thehash = UserManager.hash_password(str(random.getrandbits(256)))
thehash = UserManager.hash_password_sha512(str(random.getrandbits(256)))
self.user_manager.set_session_token(thehash)

backups = self.get_backup_list(course)
Expand Down
19 changes: 11 additions & 8 deletions inginious/frontend/pages/preferences/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import flask
from pymongo import ReturnDocument
from werkzeug.exceptions import NotFound
from argon2 import PasswordHasher
from argon2.exceptions import VerifyMismatchError

from inginious.frontend.pages.utils import INGIniousAuthPage
from inginious.frontend.user_manager import UserManager
Expand Down Expand Up @@ -54,20 +56,21 @@ def save_profile(self, userdata, data):
msg = _("Passwords don't match !")
return result, msg, error
elif self.app.allow_registration and len(data["passwd"]) >= 6:
oldpasswd_hash = UserManager.hash_password(data["oldpasswd"])
passwd_hash = UserManager.hash_password(data["passwd"])

match = {"username": self.user_manager.session_username()}
if "password" in userdata:
match["password"] = oldpasswd_hash
user = self.user_manager.auth_user(self.user_manager.session_username(), data["oldpasswd"], False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little bit confused with prior code already, but can you confirm that a user with no password (registered through social networks for instance) can actually set a password to use password authentication ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can now. I changed the structure for handling user password change in this PR. Tell me if your ok with it.

else:
user = self.database.users.find_one({"username": userdata["username"]})

result = self.database.users.find_one_and_update(match,
{"$set": {"password": passwd_hash}},
return_document=ReturnDocument.AFTER)
if not result:
if user is None:
error = True
msg = _("Incorrect old password.")
return result, msg, error
else:
passwd_hash = UserManager.hash_password(data["passwd"])
result = self.database.users.find_one_and_update({"username": self.user_manager.session_username()},
{"$set": {"password": passwd_hash}},
return_document=ReturnDocument.AFTER)

# Check if updating language
if data["language"] != userdata["language"]:
Expand Down
4 changes: 2 additions & 2 deletions inginious/frontend/pages/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def register_user(self, data):
msg = _("This email address is already in use !")
else:
passwd_hash = UserManager.hash_password(data["passwd"])
activate_hash = UserManager.hash_password(str(random.getrandbits(256)))
activate_hash = UserManager.hash_password_sha512(str(random.getrandbits(256)))
self.database.users.insert_one({"username": data["username"],
"realname": data["realname"],
"email": email,
Expand Down Expand Up @@ -138,7 +138,7 @@ def lost_passwd(self, data):
msg = _("Invalid email format.")

if not error:
reset_hash = UserManager.hash_password(str(random.getrandbits(256)))
reset_hash = UserManager.hash_password_sha512(str(random.getrandbits(256)))
user = self.database.users.find_one_and_update({"email": data["recovery_email"]},
{"$set": {"reset": reset_hash}})
if user is None:
Expand Down
76 changes: 67 additions & 9 deletions inginious/frontend/user_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from binascii import hexlify
import os
import re
from argon2 import PasswordHasher
from argon2.exceptions import VerifyMismatchError


class AuthInvalidInputException(Exception):
Expand Down Expand Up @@ -286,21 +288,55 @@ def get_auth_methods(self):
"""
return self._auth_methods

def auth_user(self, username, password):
def auth_user(self, username, password, do_connect=True):
"""
Authenticate the user in database
:param username: Username/Login
:param password: User password
:return: Returns a dict representing the user
:param do_connect: indicates if the user must be connected after authentification, True by default
:return: Returns a dict representing the user, or None if the authentication was not successful
"""
password_hash = self.hash_password(password)

user = self._database.users.find_one(
{"username": username, "password": password_hash, "activate": {"$exists": False}})
{"username": username, "activate": {"$exists": False}})

if user is None:
return None

method, db_hash = user["password"].split("-", 1) if "-" in user["password"] else ("sha512", user["password"])

if self.verify_hash(db_hash, password, method):
if do_connect:
self.connect_user(username, user["realname"], user["email"], user["language"],
user.get("tos_accepted", False))
return user

def verify_hash(cls, db_hash, password, method="sha512"):
"""
Verify a hash
:param db_hash: The hash to verify
:param password: The password to verify
:param method: The hash method
:return: A boolean if the hash is correct
"""
available_methods = {"sha512": cls.verify_hash_sha512, "argon2id": cls.verify_hash_argon2id}

if method in available_methods:
return available_methods[method](db_hash, password)
else:
raise AuthInvalidMethodException()


def verify_hash_sha512(cls, db_hash, password):
return cls.hash_password_sha512(password) == db_hash


def verify_hash_argon2id(cls, db_hash, password):
try:
ph = PasswordHasher()
return ph.verify(db_hash, password)
except VerifyMismatchError:
return False

return user if user is not None and self.connect_user(username, user["realname"], user["email"],
user["language"],
user.get("tos_accepted", False)) else None

def is_user_activated(self, username):
"""
Expand Down Expand Up @@ -1034,9 +1070,31 @@ def generate_api_key(cls):
return hexlify(os.urandom(40)).decode('utf-8')

@classmethod
def hash_password(cls, content):
def hash_password_sha512(cls, content):
"""
:param content: a str input
:return a hash of str input
"""
return hashlib.sha512(content.encode("utf-8")).hexdigest()

@classmethod
def hash_password_argon2id(cls, content):
"""
:param content: a str input
:return a hash of str input
"""
ph = PasswordHasher()
return ph.hash(content)

@classmethod
def hash_password(cls, content):
anthonygego marked this conversation as resolved.
Show resolved Hide resolved
"""
Encapsulates the other password hashing functions
:param content: a str input
:return a hash of str input
"""

methods = {"argon2id": cls.hash_password_argon2id, "sha512": cls.hash_password_sha512}
latest_method = "argon2id"

return latest_method + "-" + methods[latest_method](content)
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"Werkzeug==3.0.1",
"WsgiDAV==4.3.0",
"zipstream==1.1.4"
"argon2-cffi == 23.1.0"
]

test_requires = [
Expand Down
Loading