From 7348515e6c9b581535eb3d366cfe2bee73c5959e Mon Sep 17 00:00:00 2001 From: Kevin Chung Date: Tue, 21 Nov 2017 22:20:31 -0500 Subject: [PATCH] 34 reduce auth restrictions (#474) * Disallow email-address team names & allow login with team name or email address * Don't show password reset form if server isn't configured * Add a message to contact admins instead of submit password reset form * Add utils.check_email_format() --- CTFd/admin/teams.py | 10 ++++-- CTFd/auth.py | 36 +++++++++++++++---- CTFd/themes/original/templates/login.html | 2 +- .../original/templates/reset_password.html | 10 ++++++ CTFd/utils.py | 4 +++ CTFd/views.py | 5 ++- tests/test_utils.py | 21 +++++++++++ tests/user/test_user_facing.py | 22 ++++++++++++ 8 files changed, 100 insertions(+), 10 deletions(-) diff --git a/CTFd/admin/teams.py b/CTFd/admin/teams.py index a35dedf..60b4b1b 100644 --- a/CTFd/admin/teams.py +++ b/CTFd/admin/teams.py @@ -64,13 +64,16 @@ def admin_create_team(): elif Teams.query.filter(Teams.name == name).first(): errors.append('That name is taken') + if utils.check_email_format(name) is True: + errors.append('Team name cannot be an email address') + if not email: errors.append('The team requires an email') elif Teams.query.filter(Teams.email == email).first(): errors.append('That email is taken') if email: - valid_email = re.match(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", email) + valid_email = utils.check_email_format(email) if not valid_email: errors.append("That email address is invalid") @@ -144,7 +147,7 @@ def admin_team(teamid): errors = [] if email: - valid_email = re.match(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", email) + valid_email = utils.check_email_format(email) if not valid_email: errors.append("That email address is invalid") @@ -152,6 +155,9 @@ def admin_team(teamid): if name_used and int(name_used.id) != int(teamid): errors.append('That name is taken') + if utils.check_email_format(name) is True: + errors.append('Team name cannot be an email address') + email_used = Teams.query.filter(Teams.email == email).first() if email_used and int(email_used.id) != int(teamid): errors.append('That email is taken') diff --git a/CTFd/auth.py b/CTFd/auth.py index 3fc1c78..e73471b 100644 --- a/CTFd/auth.py +++ b/CTFd/auth.py @@ -86,7 +86,7 @@ def reset_password(data=None): except BadTimeSignature: return render_template('reset_password.html', errors=['Your link has expired']) except: - return render_template('reset_password.html', errors=['Your link appears broken, please try again.']) + return render_template('reset_password.html', errors=['Your link appears broken, please try again']) team = Teams.query.filter_by(name=name).first_or_404() team.password = bcrypt_sha256.encrypt(request.form['password'].strip()) db.session.commit() @@ -101,8 +101,20 @@ def reset_password(data=None): if request.method == 'POST': email = request.form['email'].strip() team = Teams.query.filter_by(email=email).first() + + errors = [] + + if utils.can_send_mail() is False: + return render_template( + 'reset_password.html', + errors=['Email could not be sent due to server misconfiguration'] + ) + if not team: - return render_template('reset_password.html', errors=['If that account exists you will receive an email, please check your inbox']) + return render_template( + 'reset_password.html', + errors=['If that account exists you will receive an email, please check your inbox'] + ) s = TimedSerializer(app.config['SECRET_KEY']) token = s.dumps(team.name) text = """ @@ -114,7 +126,10 @@ Did you initiate a password reset? utils.sendmail(email, text) - return render_template('reset_password.html', errors=['If that account exists you will receive an email, please check your inbox']) + return render_template( + 'reset_password.html', + errors=['If that account exists you will receive an email, please check your inbox'] + ) return render_template('reset_password.html') @@ -134,12 +149,15 @@ def register(): emails = Teams.query.add_columns('email', 'id').filter_by(email=email).first() pass_short = len(password) == 0 pass_long = len(password) > 128 - valid_email = re.match(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", request.form['email']) + valid_email = utils.check_email_format(request.form['email']) + team_name_email_check = utils.check_email_format(name) if not valid_email: - errors.append("That email doesn't look right") + errors.append("Please enter a valid email address") if names: errors.append('That team name is already taken') + if team_name_email_check is True: + errors.append('Your team name cannot be an email address') if emails: errors.append('That email has already been used') if pass_short: @@ -196,7 +214,13 @@ def login(): if request.method == 'POST': errors = [] name = request.form['name'] - team = Teams.query.filter_by(name=name).first() + + # Check if the user submitted an email address or a team name + if utils.check_email_format(name) is True: + team = Teams.query.filter_by(email=name).first() + else: + team = Teams.query.filter_by(name=name).first() + if team: if team and bcrypt_sha256.verify(request.form['password'], team.password): try: diff --git a/CTFd/themes/original/templates/login.html b/CTFd/themes/original/templates/login.html index 2f276cb..d7d5e00 100644 --- a/CTFd/themes/original/templates/login.html +++ b/CTFd/themes/original/templates/login.html @@ -36,7 +36,7 @@ diff --git a/CTFd/themes/original/templates/reset_password.html b/CTFd/themes/original/templates/reset_password.html index 1b32d83..e939fde 100644 --- a/CTFd/themes/original/templates/reset_password.html +++ b/CTFd/themes/original/templates/reset_password.html @@ -19,6 +19,7 @@ {% endfor %} + {% if can_send_mail() %}
{% if mode %} @@ -49,6 +50,15 @@
+ {% else %} +
+
+

Contact a CTF organizer

+

This CTF is not configured to send email.

+

Please contact an organizer to have your password reset

+
+
+ {% endif %} diff --git a/CTFd/utils.py b/CTFd/utils.py index a785af8..eb4fe51 100644 --- a/CTFd/utils.py +++ b/CTFd/utils.py @@ -621,6 +621,10 @@ def validate_url(url): return urlparse(url).scheme.startswith('http') +def check_email_format(email): + return bool(re.match(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", email)) + + def sha512(string): return hashlib.sha512(string).hexdigest() diff --git a/CTFd/views.py b/CTFd/views.py index 953e38f..fd5a254 100644 --- a/CTFd/views.py +++ b/CTFd/views.py @@ -220,7 +220,10 @@ def profile(): name_len = len(request.form['name']) == 0 emails = Teams.query.filter_by(email=email).first() - valid_email = re.match(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", email) + valid_email = utils.check_email_format(email) + + if utils.check_email_format(name) is True: + errors.append('Team name cannot be an email address') if ('password' in request.form.keys() and not len(request.form['password']) == 0) and \ (not bcrypt_sha256.verify(request.form.get('confirm').strip(), user.password)): diff --git a/tests/test_utils.py b/tests/test_utils.py index c8f0cdb..5a42b5b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -6,6 +6,7 @@ from CTFd.models import ip2long, long2ip from CTFd.utils import get_config, set_config, override_template, sendmail, verify_email, ctf_started, ctf_ended, export_ctf from CTFd.utils import register_plugin_script, register_plugin_stylesheet from CTFd.utils import base64encode, base64decode +from CTFd.utils import check_email_format from freezegun import freeze_time from mock import patch import json @@ -349,3 +350,23 @@ def test_export_ctf(): with open('export.zip', 'wb') as f: f.write(backup.getvalue()) destroy_ctfd(app) + + +def test_check_email_format(): + """Test that the check_email_format() works properly""" + assert check_email_format('user@ctfd.io') is True + assert check_email_format('user+plus@gmail.com') is True + assert check_email_format('user.period1234@gmail.com') is True + assert check_email_format('user.period1234@b.c') is True + assert check_email_format('user.period1234@b') is False + assert check_email_format('no.ampersand') is False + assert check_email_format('user@') is False + assert check_email_format('@ctfd.io') is False + assert check_email_format('user.io@ctfd') is False + assert check_email_format('user\@ctfd') is False + + for invalid_email in ['user.@ctfd.io', '.user@ctfd.io', 'user@ctfd..io']: + try: + assert check_email_format(invalid_email) is False + except AssertionError: + print(invalid_email, 'did not pass validation') diff --git a/tests/user/test_user_facing.py b/tests/user/test_user_facing.py index 9f3dd5b..ab209a4 100644 --- a/tests/user/test_user_facing.py +++ b/tests/user/test_user_facing.py @@ -40,6 +40,16 @@ def test_register_unicode_user(): destroy_ctfd(app) +def test_register_email_as_team_name(): + """A user shouldn't be able to use an email address as a team name""" + app = create_ctfd() + with app.app_context(): + register_user(app, name="user@ctfd.io", email="user@ctfd.io", password="password") + team_count = app.db.session.query(app.db.func.count(Teams.id)).first()[0] + assert team_count == 1 # There's only the admin user + destroy_ctfd(app) + + def test_register_duplicate_teamname(): """A user shouldn't be able to use an already registered team name""" app = create_ctfd() @@ -85,6 +95,18 @@ def test_user_login(): destroy_ctfd(app) +def test_user_login_with_email(): + """Can a registered user can login with an email address instead of a team name""" + app = create_ctfd() + with app.app_context(): + register_user(app) + client = login_as_user(app, name="user@ctfd.io", password="password") + r = client.get('/profile') + assert r.location != "http://localhost/login" # We didn't get redirected to login + assert r.status_code == 200 + destroy_ctfd(app) + + def test_user_isnt_admin(): """A registered user cannot access admin pages""" app = create_ctfd()