Repository: CTFd/CTFd
Pull request #525 (2017-12-14 02:36:06)

Adding an error message for invalid confirm/reset links 
Adds some better messaging for invalid confirmation & email links. 
Also adds tests for invalid tokens. 

codecov[bot]: # [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/525?src=pr&el=h1) Report
> Merging [#525](https://codecov.io/gh/CTFd/CTFd/pull/525?src=pr&el=desc) into [master](https://codecov.io/gh/CTFd/CTFd/commit/90b4ee03ad3070e5dc983bb1dee761e7e8b0aee2?src=pr&el=desc) will **increase** coverage by `0.24%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/CTFd/CTFd/pull/525/graphs/tree.svg?height=150&width=650&src=pr&token=IfPqkWkt9A)](https://codecov.io/gh/CTFd/CTFd/pull/525?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #525      +/-   ##
==========================================
+ Coverage   71.42%   71.67%   +0.24%     
==========================================
  Files          18       18              
  Lines        2642     2644       +2     
==========================================
+ Hits         1887     1895       +8     
+ Misses        755      749       -6
```


| [Impacted Files](https://codecov.io/gh/CTFd/CTFd/pull/525?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [CTFd/auth.py](https://codecov.io/gh/CTFd/CTFd/pull/525/diff?src=pr&el=tree#diff-Q1RGZC9hdXRoLnB5) | `85.71% <100%> (+5.36%)` | :arrow_up: |
| [CTFd/utils.py](https://codecov.io/gh/CTFd/CTFd/pull/525/diff?src=pr&el=tree#diff-Q1RGZC91dGlscy5weQ==) | `80.67% <0%> (-0.51%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/CTFd/CTFd/pull/525?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/525?src=pr&el=footer). Last update [90b4ee0...72f3514](https://codecov.io/gh/CTFd/CTFd/pull/525?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CTFd/auth.py b/CTFd/auth.py
index d3e708e3..e797b42a 100644
--- a/CTFd/auth.py
+++ b/CTFd/auth.py
@@ -11,6 +11,8 @@ from CTFd.models import db, Teams
 from CTFd import utils
 from CTFd.utils import ratelimit
 
+import base64
+
 auth = Blueprint('auth', __name__)
 
 
@@ -30,8 +32,8 @@ def confirm_user(data=None):
             email = s.loads(utils.base64decode(data, urldecode=True), max_age=1800)
         except BadTimeSignature:
             return render_template('confirm.html', errors=['Your confirmation link has expired'])
-        except BadSignature:
-            return render_template('confirm.html', errors=['Your confirmation link seems wrong'])
+        except (BadSignature, TypeError, base64.binascii.Error):
+            return render_template('confirm.html', errors=['Your confirmation token is invalid'])
         team = Teams.query.filter_by(email=email).first_or_404()
         team.verified = True
         db.session.commit()
@@ -80,26 +82,29 @@ def confirm_user(data=None):
 @ratelimit(method="POST", limit=10, interval=60)
 def reset_password(data=None):
     logger = logging.getLogger('logins')
-    if data is not None and request.method == "GET":
-        return render_template('reset_password.html', mode='set')
-    if data is not None and request.method == "POST":
+
+    if data is not None:
         try:
             s = TimedSerializer(app.config['SECRET_KEY'])
             name = s.loads(utils.base64decode(data, urldecode=True), max_age=1800)
         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'])
-        team = Teams.query.filter_by(name=name).first_or_404()
-        team.password = bcrypt_sha256.encrypt(request.form['password'].strip())
-        db.session.commit()
-        logger.warn("[{date}] {ip} -  successful password reset for {username}".format(
-            date=time.strftime("%m/%d/%Y %X"),
-            ip=utils.get_ip(),
-            username=team.name.encode('utf-8')
-        ))
-        db.session.close()
-        return redirect(url_for('auth.login'))
+        except (BadSignature, TypeError, base64.binascii.Error):
+            return render_template('reset_password.html', errors=['Your reset token is invalid'])
+
+        if request.method == "GET":
+            return render_template('reset_password.html', mode='set')
+        if request.method == "POST":
+            team = Teams.query.filter_by(name=name).first_or_404()
+            team.password = bcrypt_sha256.encrypt(request.form['password'].strip())
+            db.session.commit()
+            logger.warn("[{date}] {ip} -  successful password reset for {username}".format(
+                date=time.strftime("%m/%d/%Y %X"),
+                ip=utils.get_ip(),
+                username=team.name.encode('utf-8')
+            ))
+            db.session.close()
+            return redirect(url_for('auth.login'))
 
     if request.method == 'POST':
         email = request.form['email'].strip()
diff --git a/CTFd/themes/core/templates/confirm.html b/CTFd/themes/core/templates/confirm.html
index 7cce0962..1a2ccc29 100644
--- a/CTFd/themes/core/templates/confirm.html
+++ b/CTFd/themes/core/templates/confirm.html
@@ -46,7 +46,7 @@
 						Need to resend the confirmation email?
 					</h4>
 					<div class="col-md-12 text-center">
-						<button type="submit" id="submit" tabindex="5" class="btn btn-md btn-primary btn-outlined float-right">Resend</button>
+						<button type="submit" id="submit" tabindex="5" class="btn btn-md btn-primary btn-outlined">Resend</button>
 					</div>
 					<input type="hidden" name="nonce" value="{{ nonce }}">
 				</form>
diff --git a/CTFd/utils.py b/CTFd/utils.py
index d6470e16..d000e5ab 100644
--- a/CTFd/utils.py
+++ b/CTFd/utils.py
@@ -699,7 +699,10 @@ def base64encode(s, urlencode=False):
 
     encoded = base64.urlsafe_b64encode(s)
     if six.PY3:
-        encoded = encoded.decode('utf-8')
+        try:
+            encoded = encoded.decode('utf-8')
+        except UnicodeDecodeError:
+            pass
     if urlencode:
         encoded = quote(encoded)
     return encoded
@@ -717,7 +720,10 @@ def base64decode(s, urldecode=False):
 
     decoded = base64.urlsafe_b64decode(s)
     if six.PY3:
-        decoded = decoded.decode('utf-8')
+        try:
+            decoded = decoded.decode('utf-8')
+        except UnicodeDecodeError:
+            pass
     return decoded
 
 


Test patch:
diff --git a/tests/user/test_auth.py b/tests/user/test_auth.py
index 1db47623..cf08a6c2 100644
--- a/tests/user/test_auth.py
+++ b/tests/user/test_auth.py
@@ -103,3 +103,97 @@ def test_verify_and_view_unregistered():
         r = client.get('/chals')
         assert r.status_code == 200
     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():
+        set_config('verify_emails', True)
+
+        register_user(app, email="user@user.com")
+        client = login_as_user(app, name="user", password="password")
+
+        # user@user.com "2012-01-14 03:21:34"
+        confirm_link = 'http://localhost/confirm/InVzZXJAdXNlci5jb20iLkFmS0dQZy5kLUJnVkgwaUhadzFHaXVENHczWTJCVVJwdWc%3D'
+        r = client.get(confirm_link)
+
+        assert "Your confirmation link has expired" in r.get_data(as_text=True)
+        team = Teams.query.filter_by(email='user@user.com').first()
+        assert team.verified is not True
+    destroy_ctfd(app)
+
+
+def test_invalid_confirmation_links():
+    """Test that invalid confirmation links are reported to the user"""
+    app = create_ctfd()
+    with app.app_context():
+        set_config('verify_emails', True)
+
+        register_user(app, email="user@user.com")
+        client = login_as_user(app, name="user", password="password")
+
+        # user@user.com "2012-01-14 03:21:34"
+        confirm_link = 'http://localhost/confirm/a8375iyu<script>alert(1)<script>hn3048wueorighkgnsfg%3D%3D'
+        r = client.get(confirm_link)
+
+        assert "Your confirmation token is invalid" in r.get_data(as_text=True)
+        team = Teams.query.filter_by(email='user@user.com').first()
+        assert team.verified is not True
+    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()
+    with app.app_context():
+        set_config('mail_server', 'localhost')
+        set_config('mail_port', 25)
+        set_config('mail_username', 'username')
+        set_config('mail_password', 'password')
+
+        register_user(app, name="user1", email="user@user.com")
+
+        with app.test_client() as client:
+            # user@user.com "2012-01-14 03:21:34"
+            forgot_link = 'http://localhost/reset_password/InVzZXIxIi5BZktHUGcuTVhkTmZtOWU2U2xwSXZ1MlFwTjdwa3F5V3hR'
+            r = client.get(forgot_link)
+
+            assert "Your link has expired" in r.get_data(as_text=True)
+    destroy_ctfd(app)
+
+
+def test_invalid_reset_password_link():
+    """Test that invalid reset password links are reported to the user"""
+    app = create_ctfd()
+    with app.app_context():
+        set_config('mail_server', 'localhost')
+        set_config('mail_port', 25)
+        set_config('mail_username', 'username')
+        set_config('mail_password', 'password')
+
+        register_user(app, name="user1", email="user@user.com")
+
+        with app.test_client() as client:
+            # user@user.com "2012-01-14 03:21:34"
+            forgot_link = 'http://localhost/reset_password/5678ytfghjiu876tyfg<>hvbnmkoi9u87y6trdfcgvhbnm,lp09iujmk%3D'
+            r = client.get(forgot_link)
+
+            assert "Your reset token is invalid" in r.get_data(as_text=True)
+    destroy_ctfd(app)
+
+
+def test_contact_for_password_reset():
+    """Test that if there is no mailserver configured, users should contact admins"""
+    app = create_ctfd()
+    with app.app_context():
+        register_user(app, name="user1", email="user@user.com")
+
+        with app.test_client() as client:
+            forgot_link = 'http://localhost/reset_password'
+            r = client.get(forgot_link)
+
+            assert "Contact a CTF organizer" in r.get_data(as_text=True)
+    destroy_ctfd(app)
diff --git a/tests/user/test_user_facing.py b/tests/user/test_user_facing.py
index b201e179..c3b1ec0b 100644
--- a/tests/user/test_user_facing.py
+++ b/tests/user/test_user_facing.py
@@ -580,7 +580,7 @@ def test_user_can_confirm_email(mock_smtp):
 @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'''
+    """Test that a user is capable of resetting their password"""
     from email.mime.text import MIMEText
     app = create_ctfd()
     with app.app_context():
@@ -635,7 +635,7 @@ http://localhost/reset_password/InVzZXIxIi5BZktHUGcuTVhkTmZtOWU2U2xwSXZ1MlFwTjdw
                 }
 
             # Do the password reset
-            r = client.get('/reset_password')
+            r = client.get('/reset_password/InVzZXIxIi5BZktHUGcuTVhkTmZtOWU2U2xwSXZ1MlFwTjdwa3F5V3hR')
             r = client.post('/reset_password/InVzZXIxIi5BZktHUGcuTVhkTmZtOWU2U2xwSXZ1MlFwTjdwa3F5V3hR', data=data)
 
             # Make sure that the user's password changed