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

Centralize password check as method of 'User' objects #341

Merged
merged 2 commits into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 KerbalStuff/blueprints/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def login() -> Union[str, werkzeug.wrappers.Response]:
return render_template("login.html", username=username, errors='Your username or password is incorrect.')
if user.confirmation != '' and user.confirmation is not None:
return redirect("/account-pending")
if not bcrypt.hashpw(password.encode('utf-8'), user.password.encode('utf-8')) == user.password.encode('utf-8'):
if not user.check_password(password):
return render_template("login.html", username=username, errors='Your username or password is incorrect.')
login_user(user, remember=remember)
if 'return_to' in request.form and request.form['return_to']:
Expand Down
5 changes: 2 additions & 3 deletions KerbalStuff/blueprints/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ def login() -> Union[Dict[str, Any], Tuple[Dict[str, Any], int]]:
user = User.query.filter(User.username.ilike(username)).first()
if not user:
return {'error': True, 'reason': 'Username or password is incorrect'}, 401
if not bcrypt.hashpw(password.encode('utf-8'),
user.password.encode('utf-8')) == user.password.encode('utf-8'):
if not user.check_password(password):
return {'error': True, 'reason': 'Username or password is incorrect'}, 401
if user.confirmation and user.confirmation is not None:
return {'error': True, 'reason': 'User is not confirmed'}, 403
Expand Down Expand Up @@ -439,7 +438,7 @@ def change_password(username: str) -> Union[Dict[str, Any], Tuple[Union[str, Any
new_password = request.form.get('new-password', '')
new_password_confirm = request.form.get('new-password-confirm', '')

if not bcrypt.hashpw(old_password.encode('utf-8'), current_user.password.encode('utf-8')) == current_user.password.encode('utf-8'):
if not current_user.check_password(old_password):
return {'error': True, 'reason': 'The old password you entered doesn\'t match your current account password.'}

pw_valid, pw_message = check_password_criteria(new_password, new_password_confirm)
Expand Down
3 changes: 3 additions & 0 deletions KerbalStuff/objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ class User(Base): # type: ignore
def set_password(self, password: str) -> None:
self.password = bcrypt.hashpw(password.encode('utf-8'), bcrypt.gensalt()).decode('utf-8')

def check_password(self, password: str) -> bool:
return bcrypt.checkpw(password.encode('utf-8'), self.password.encode('utf-8'))

def create_confirmation(self) -> None:
self.confirmation = binascii.b2a_hex(os.urandom(20)).decode('utf-8')

Expand Down
2 changes: 1 addition & 1 deletion requirements-backend.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
alembic
bcrypt
bcrypt>=3.1.0
bleach
bleach-allowlist
celery
Expand Down
3 changes: 2 additions & 1 deletion tests/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from .test_version import *
from .test_api_browse import *
from .test_api_mod import *
from .test_api_errors import *
from .test_errors import *
from .test_objects_user import *
from .test_version import *
17 changes: 17 additions & 0 deletions tests/test_objects_user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from KerbalStuff.objects import User


def test_objects_user() -> None:
# Arrange
# Create new user
user = User(username="Test User", email="test_user@example.com")
password_clear_text = "deRp*qnEX&<6i`=<oFy3j+%ww3<-k*:4"
user.set_password(password_clear_text)

# Act
correct_password_match = user.check_password(password_clear_text)
incorrect_password_match = user.check_password("password1!")

# Assert
assert correct_password_match is True, "user.check_password should return True for matching passwords"
assert incorrect_password_match is False, "user.check_password should return False for nonmatching passwords"