Various fixes (#1733)

* Reset all password tokens on password reset

* Added csrf validation on email change request and validation

* Return the same wether is a valid email or not

---------

Co-authored-by: Adrià Casajús <adria.casajus@proton.ch>

app/auth/views/forgot_password.py

@@ -1,4 +1,4 @@
-from flask import request, render_template, redirect, url_for, flash, g
+from flask import request, render_template, flash, g
 from flask_wtf import FlaskForm
 from wtforms import StringField, validators
 
@@ -16,7 +16,7 @@ class ForgotPasswordForm(FlaskForm):
 
 @auth_bp.route("/forgot_password", methods=["GET", "POST"])
 @limiter.limit(
-    "10/minute", deduct_when=lambda r: hasattr(g, "deduct_limit") and g.deduct_limit
+    "10/hour", deduct_when=lambda r: hasattr(g, "deduct_limit") and g.deduct_limit
 )
 def forgot_password():
     form = ForgotPasswordForm(request.form)
@@ -37,6 +37,5 @@ def forgot_password():
         if user:
             LOG.d("Send forgot password email to %s", user)
             send_reset_password_email(user)
-            return redirect(url_for("auth.forgot_password"))
 
     return render_template("auth/forgot_password.html", form=form)

app/auth/views/reset_password.py

@@ -60,8 +60,8 @@ def reset_password():
         # this can be served to activate user too
         user.activated = True
 
-        # remove the reset password code
-        ResetPasswordCode.delete(reset_password_code.id)
+        # remove all reset password codes
+        ResetPasswordCode.filter_by(user_id=user.id).delete()
 
         # change the alternative_id to log user out on other browsers
         user.alternative_id = str(uuid.uuid4())

app/dashboard/views/setting.py

@@ -198,6 +198,16 @@ def setting():
                         )
                         return redirect(url_for("dashboard.setting"))
 
+                    if current_user.profile_picture_id is not None:
+                        current_profile_file = File.get_by(
+                            id=current_user.profile_picture_id
+                        )
+                        if (
+                            current_profile_file is not None
+                            and current_profile_file.user_id == current_user.id
+                        ):
+                            s3.delete(current_user.path)
+
                     file_path = random_string(30)
                     file = File.create(user_id=current_user.id, path=file_path)
 
@@ -451,8 +461,13 @@ def send_change_email_confirmation(user: User, email_change: EmailChange):
 
 
 @dashboard_bp.route("/resend_email_change", methods=["GET", "POST"])
+@limiter.limit("5/hour")
 @login_required
 def resend_email_change():
+    form = CSRFValidationForm()
+    if not form.validate():
+        flash("Invalid request. Please try again", "warning")
+        return redirect(url_for("dashboard.setting"))
     email_change = EmailChange.get_by(user_id=current_user.id)
     if email_change:
         # extend email change expiration
@@ -472,6 +487,10 @@ def resend_email_change():
 @dashboard_bp.route("/cancel_email_change", methods=["GET", "POST"])
 @login_required
 def cancel_email_change():
+    form = CSRFValidationForm()
+    if not form.validate():
+        flash("Invalid request. Please try again", "warning")
+        return redirect(url_for("dashboard.setting"))
     email_change = EmailChange.get_by(user_id=current_user.id)
     if email_change:
         EmailChange.delete(email_change.id)

templates/dashboard/setting.html

@@ -181,10 +181,10 @@
     <!-- END change name & profile picture -->
     <!-- Change email -->
     <div class="card">
-      <form method="post" enctype="multipart/form-data">
-        <input type="hidden" name="form-name" value="update-email">
-        {{ change_email_form.csrf_token }}
-        <div class="card-body">
+      <div class="card-body">
+        <form method="post" enctype="multipart/form-data">
+          <input type="hidden" name="form-name" value="update-email">
+          {{ change_email_form.csrf_token }}
           <div class="card-title">Account Email</div>
           <div class="mb-3">
             This email address is used to log in to SimpleLogin.
@@ -199,26 +199,30 @@
             <!-- Not allow user to change email if there's a pending change -->
             {{ change_email_form.email(class="form-control", value=current_user.email, readonly=pending_email != None) }}
             {{ render_field_errors(change_email_form.email) }}
-            {% if pending_email %}
-
-              <div class="mt-2">
-                <span class="text-danger">Pending email change: {{ pending_email }}</span>
-                <a href="{{ url_for('dashboard.resend_email_change') }}"
-                   class="btn btn-secondary btn-sm">
-                  Resend
-                  confirmation email
-                </a>
-                <a href="{{ url_for('dashboard.cancel_email_change') }}"
-                   class="btn btn-secondary btn-sm">
-                  Cancel email
-                  change
-                </a>
-              </div>
-            {% endif %}
           </div>
           <button class="btn btn-outline-primary">Change Email</button>
-        </div>
-      </form>
+        </form>
+        {% if pending_email %}
+
+          <div class="mt-2">
+            <span class="text-danger float-left">Pending email change: {{ pending_email }}</span>
+            <form method="POST"
+                  action="{{ url_for('dashboard.resend_email_change') }}"
+                  class="float-left ml-2">
+              {{ change_email_form.csrf_token }}
+              <a onclick="this.closest('form').submit()"
+                 class="btn btn-secondary btn-sm">Resend confirmation email</a>
+            </form>
+            <form method="POST"
+                  action="{{ url_for('dashboard.cancel_email_change') }}"
+                  class="float-left ml-2">
+              {{ change_email_form.csrf_token }}
+              <a onclick="this.closest('form').submit()"
+                 class="btn btn-secondary btn-sm">Cancel email change</a>
+            </form>
+          </div>
+        {% endif %}
+      </div>
     </div>
     <!-- END Change email -->
     <!-- Connect with Proton -->
@@ -265,11 +269,15 @@
     <div class="card" id="change_password">
       <div class="card-body">
         <div class="card-title">Password</div>
-        <div class="mb-3">You will receive an email containing instructions on how to change your password.</div>
+        <div class="mb-3">
+          You will receive an email containing instructions on how to change your password.
+        </div>
         <form method="post">
           {{ csrf_form.csrf_token }}
           <input type="hidden" name="form-name" value="change-password">
-          <button class="btn btn-outline-primary">Change password</button>
+          <button class="btn btn-outline-primary">
+            Change password
+          </button>
         </form>
       </div>
     </div>

tests/auth/test_reset_password.py

@@ -0,0 +1,26 @@
+from flask import url_for
+
+from app.db import Session
+from app.models import User, ResetPasswordCode
+from tests.utils import create_new_user, random_token
+
+
+def test_reset_password(flask_client):
+    user = create_new_user()
+    original_pass_hash = user.password
+    user_id = user.id
+    reset_code = random_token()
+    ResetPasswordCode.create(user_id=user.id, code=reset_code)
+    ResetPasswordCode.create(user_id=user.id, code=random_token())
+    Session.commit()
+
+    r = flask_client.post(
+        url_for("auth.reset_password", code=reset_code),
+        data={"password": "1231idsfjaads"},
+    )
+
+    assert r.status_code == 200
+
+    assert len(ResetPasswordCode.get_by(user_id=user_id).all()) == 0
+    user = User.get(user_id)
+    assert user.password != original_pass_hash