Fix email confirmations and password resets (#795)

* Fix email confirmations and improve test.
* Breaks confirm.html page because of change from team to user
* Clean up admin mail settings to use new label/small structure
* Fix password resets from double hashing passwords

CTFd/auth.py

@@ -7,8 +7,6 @@ from flask import (
     session,
     Blueprint,
 )
-from passlib.hash import bcrypt_sha256
-
 from CTFd.models import db, Users, Teams
 
 from CTFd.utils import get_config, get_app_config
@@ -17,6 +15,7 @@ from CTFd.utils import user as current_user
 from CTFd.utils import config, validators
 from CTFd.utils import email
 from CTFd.utils.security.auth import login_user, logout_user
+from CTFd.utils.security.passwords import hash_password, check_password
 from CTFd.utils.logging import log
 from CTFd.utils.decorators.visibility import check_registration_visibility
 from CTFd.utils.modes import TEAMS_MODE, USERS_MODE
@@ -46,8 +45,8 @@ def confirm(data=None):
         except (BadSignature, TypeError, base64.binascii.Error):
             return render_template('confirm.html', errors=['Your confirmation token is invalid'])
 
-        team = Users.query.filter_by(email=user_email).first_or_404()
+        user = Users.query.filter_by(email=user_email).first_or_404()
-        team.verified = True
+        user.verified = True
         log('registrations', format="[{date}] {ip} -  successful password reset for {name}")
         db.session.commit()
         db.session.close()
@@ -59,24 +58,19 @@ def confirm(data=None):
     if not current_user.authed():
         return redirect(url_for('auth.login'))
 
-    team = Users.query.filter_by(id=session['id']).first_or_404()
+    user = Users.query.filter_by(id=session['id']).first_or_404()
+    if user.verified:
+        return redirect(url_for('views.settings'))
 
     if data is None:
         if request.method == "POST":
             # User wants to resend their confirmation email
-            if team.verified:
+            email.verify_email_address(user.email)
-                return redirect(url_for('views.profile'))
+            log('registrations', format="[{date}] {ip} - {name} initiated a confirmation email resend")
-            else:
+            return render_template('confirm.html', user=user, infos=['Your confirmation email has been resent!'])
-                email.verify_email_address(team.email)
-                log('registrations', format="[{date}] {ip} - {name} initiated a confirmation email resend")
-            return render_template('confirm.html', team=team, infos=['Your confirmation email has been resent!'])
         elif request.method == "GET":
             # User has been directed to the confirm page
-            team = Users.query.filter_by(id=session['id']).first_or_404()
+            return render_template('confirm.html', user=user)
-            if team.verified:
-                # If user is already verified, redirect to their profile
-                return redirect(url_for('views.profile'))
-            return render_template('confirm.html', team=team)
 
 
 @auth.route('/reset_password', methods=['POST', 'GET'])
@@ -94,10 +88,10 @@ def reset_password(data=None):
         if request.method == "GET":
             return render_template('reset_password.html', mode='set')
         if request.method == "POST":
-            team = Users.query.filter_by(name=name).first_or_404()
+            user = Users.query.filter_by(name=name).first_or_404()
-            team.password = bcrypt_sha256.encrypt(request.form['password'].strip())
+            user.password = request.form['password'].strip()
             db.session.commit()
-            log('logins', format="[{date}] {ip} -  successful password reset for {name}")
+            log('logins', format="[{date}] {ip} -  successful password reset for {name}", name=name)
             db.session.close()
             return redirect(url_for('auth.login'))
 
@@ -226,7 +220,7 @@ def login():
             user = Users.query.filter_by(name=name).first()
 
         if user:
-            if user and bcrypt_sha256.verify(request.form['password'], user.password):
+            if user and check_password(request.form['password'], user.password):
                 session.regenerate()
 
                 login_user(user)

CTFd/themes/admin/templates/configs/email.html

@@ -11,26 +11,37 @@
 		</ul>
 
 		<div class="form-group">
-			<label>Mail From Address:</label>
+			<label>
-			<input class="form-control" id='mailfrom_addr' name='mailfrom_addr' type='text'
+				Mail From Address<br>
-				   placeholder="Email address used to send email"
+				<small class="form-text text-muted">
-				   {% if mailfrom_addr is defined and mailfrom_addr != None %}value="{{ mailfrom_addr }}"{% endif %}>
+					Email address used to send email
+				</small>
+			</label>
+			<input class="form-control" id='mailfrom_addr' name='mailfrom_addr' type='text' {% if mailfrom_addr is defined and mailfrom_addr != None %}value="{{ mailfrom_addr }}"{% endif %}>
 		</div>
 
 		<div class="tab-content">
 			<div role="tabpanel" class="tab-pane active" id="mailserver">
 
 				<div class="form-group">
-					<label>Mail Server Address:</label>
+					<label>
+						Mail Server Address<br>
+						<small class="form-text text-muted">
+							Mail Server Address
+						</small>
+					</label>
 					<input class="form-control" id='mail_server' name='mail_server' type='text'
-						   placeholder="Mail Server Address"
 						   {% if mail_server is defined and mail_server != None %}value="{{ mail_server }}"{% endif %}>
 				</div>
 
 				<div class="form-group">
-					<label>Mail Server Port:</label>
+					<label>
+						Mail Server Port<br>
+						<small class="form-text text-muted">
+							Mail Server Port
+						</small>
+					</label>
 					<input class="form-control" id='mail_port' name='mail_port' type='text'
-						   placeholder="Mail Server Port"
 						   {% if mail_port is defined and mail_port != None %}value="{{ mail_port }}"{% endif %}>
 				</div>
 				<div class="form-check">
@@ -42,20 +53,32 @@
 				</div>
 				<div id="mail_username_password">
 					<div class="form-group">
-						<label>Username:</label>
+						<label>
+							Username<br>
+							<small class="form-text text-muted">
+								Mail Server Account Username
+							</small>
+						</label>
 						{% if mail_username is defined and mail_username != None %}
-							<label for="mail_username"><sup>A mail server username is currently set</sup></label>
+							<label>
+								<sup class="form-text text-muted">A mail server username is currently set</sup>
+							</label>
 						{% endif %}
-						<input class="form-control" id='mail_username' name='mail_username' autocomplete='off' type='text'
+						<input class="form-control" id='mail_username' name='mail_username' autocomplete='off' type='text'>
-							   placeholder="Username">
 					</div>
 					<div class="form-group">
-						<label for="mail_password">Password:</label>
+						<label for="mail_password">
+							Password<br>
+							<small class="form-text text-muted">
+								Mail Server Account Password
+							</small>
+						</label>
 						{% if mail_password is defined and mail_password != None %}
-							<label for="mail_p"><sup>An mail server password is currently set</sup></label>
+							<label>
+								<sup class="form-text text-muted">An mail server password is currently set</sup>
+							</label>
 						{% endif %}
-						<input class="form-control" id='mail_password' name='mail_password' autocomplete='off' type='password'
+						<input class="form-control" id='mail_password' name='mail_password' autocomplete='off' type='password'>
-							   placeholder="Password">
 					</div>
 					<sup>Uncheck setting and update to remove username and password</sup>
 					<br>
@@ -76,15 +99,23 @@
 			</div>
 			<div role="tabpanel" class="tab-pane" id="mailgun">
 				<div class="form-group">
-					<label>Mailgun API Base URL:</label>
+					<label>
+						Mailgun API Base URL<br>
+						<small class="form-text text-muted">
+							Mailgun API Base URL
+						</small>
+					</label>
 					<input class="form-control" id='mailgun_base_url' name='mailgun_base_url' type='text'
-						   placeholder="Mailgun API Base URL"
 						   {% if mailgun_base_url is defined and mailgun_base_url != None %}value="{{ mailgun_base_url }}"{% endif %}>
 				</div>
 				<div class="form-group">
-					<label>Mailgun API Key:</label>
+					<label>
+						Mailgun API Key<br>
+						<small class="form-text text-muted">
+							Mailgun API Key
+						</small>
+					</label>
 					<input class="form-control" id='mailgun_api_key' name='mailgun_api_key' type='text'
-						   placeholder="Mailgun API Key"
 						   {% if mailgun_api_key is defined and mailgun_api_key != None %}value="{{ mailgun_api_key }}"{% endif %}>
 				</div>
 			</div>

CTFd/themes/core/templates/confirm.html

@@ -28,9 +28,9 @@
 								aria-hidden="true">×</span></button>
 					</div>
 				{% endfor %}
-				{% if team %}
+				{% if user %}
 				<h3 class="text-center">
-					We've sent a confirmation email to {{ team.email }}
+					We've sent a confirmation email to {{ user.email }}
 				</h3>
 
 				<br>
@@ -40,7 +40,9 @@
 				</h4>
 				{% endif %}
 
-				{% if username %}
+				<hr>
+
+				{% if name %}
 				<form method="POST" action="{{ url_for('auth.confirm') }}">
 					<h4 class="text-center">
 						Need to resend the confirmation email?

tests/users/test_auth.py

@@ -4,6 +4,7 @@
 from CTFd.models import Users
 from CTFd.utils import set_config, get_config
 from CTFd.utils.security.signing import serialize
+from CTFd.utils.security.passwords import hash_password, check_password
 from freezegun import freeze_time
 from tests.helpers import *
 from mock import patch
@@ -133,11 +134,10 @@ def test_user_isnt_admin():
     destroy_ctfd(app)
 
 
-@freeze_time("2019-02-24 03:21:34")
 def test_expired_confirmation_links():
     """Test that expired confirmation links are reported to the user"""
     app = create_ctfd()
-    with app.app_context():
+    with app.app_context(), freeze_time("2019-02-24 03:21:34"):
         set_config('verify_emails', True)
 
         register_user(app, email="user@user.com")
@@ -172,7 +172,6 @@ def test_invalid_confirmation_links():
     destroy_ctfd(app)
 
 
-@freeze_time("2019-02-24 03:21:34")
 def test_expired_reset_password_link():
     """Test that expired reset password links are reported to the user"""
     app = create_ctfd()
@@ -185,7 +184,7 @@ def test_expired_reset_password_link():
 
         register_user(app, name="user1", email="user@user.com")
 
-        with app.test_client() as client:
+        with app.test_client() as client, freeze_time("2019-02-24 03:21:34"):
             # user@user.com "2012-01-14 03:21:34"
             forgot_link = 'http://localhost/reset_password/InVzZXJAdXNlci5jb20i.TxD0vg.cAGwAy8cK1T0saEEbrDEBVF2plI'
             r = client.get(forgot_link)
@@ -230,11 +229,10 @@ def test_contact_for_password_reset():
 
 
 @patch('smtplib.SMTP')
-@freeze_time("2012-01-14 03:21:34")
 def test_user_can_confirm_email(mock_smtp):
     """Test that a user is capable of confirming their email address"""
     app = create_ctfd()
-    with app.app_context():
+    with app.app_context(), freeze_time("2012-01-14 03:21:34"):
         # Set CTFd to only allow confirmed users and send emails
         set_config('verify_emails', True)
         set_config('mail_server', 'localhost')
@@ -251,6 +249,9 @@ def test_user_can_confirm_email(mock_smtp):
 
         client = login_as_user(app, name="user1", password="password")
 
+        r = client.get('http://localhost/confirm')
+        assert "Need to resend the confirmation email?" in r.get_data(as_text=True)
+
         # smtp.sendmail was called
         mock_smtp.return_value.sendmail.assert_called()
 
@@ -258,6 +259,9 @@ def test_user_can_confirm_email(mock_smtp):
             data = {
                 "nonce": sess.get('nonce')
             }
+            r = client.post('http://localhost/confirm', data=data)
+            assert "confirmation email has been resent" in r.get_data(as_text=True)
+
             r = client.get('/challenges')
             assert r.location == "http://localhost/confirm"  # We got redirected to /confirm
 
@@ -269,16 +273,18 @@ def test_user_can_confirm_email(mock_smtp):
             # The team is now verified
             user = Users.query.filter_by(email='user@user.com').first()
             assert user.verified is True
+
+            r = client.get('http://localhost/confirm')
+            assert r.location == "http://localhost/settings"
     destroy_ctfd(app)
 
 
 @patch('smtplib.SMTP')
-@freeze_time("2012-01-14 03:21:34")
 def test_user_can_reset_password(mock_smtp):
     """Test that a user is capable of resetting their password"""
     from email.mime.text import MIMEText
     app = create_ctfd()
-    with app.app_context():
+    with app.app_context(), freeze_time("2012-01-14 03:21:34"):
         # Set CTFd to send emails
         set_config('mail_server', 'localhost')
         set_config('mail_port', 25)
@@ -333,7 +339,7 @@ def test_user_can_reset_password(mock_smtp):
 
             # Make sure that the user's password changed
             user = Users.query.filter_by(email="user@user.com").first()
-            assert user.password != user_password_saved
+            assert check_password('passwordtwo', user.password)
     destroy_ctfd(app)