From e4d43179880c14b34ed92ea533be2a9acc2ad061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Wed, 10 May 2023 15:31:30 +0200 Subject: [PATCH] Various fixes (#1733) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- app/auth/views/forgot_password.py | 5 ++- app/auth/views/reset_password.py | 4 +-- app/dashboard/views/setting.py | 19 +++++++++++ templates/dashboard/setting.html | 56 ++++++++++++++++++------------- tests/auth/test_reset_password.py | 26 ++++++++++++++ 5 files changed, 81 insertions(+), 29 deletions(-) create mode 100644 tests/auth/test_reset_password.py diff --git a/app/auth/views/forgot_password.py b/app/auth/views/forgot_password.py index 684fd383..42246a1f 100644 --- a/app/auth/views/forgot_password.py +++ b/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) diff --git a/app/auth/views/reset_password.py b/app/auth/views/reset_password.py index d3e9f74a..e331cd9f 100644 --- a/app/auth/views/reset_password.py +++ b/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()) diff --git a/app/dashboard/views/setting.py b/app/dashboard/views/setting.py index 7ad4899f..79ccb16c 100644 --- a/app/dashboard/views/setting.py +++ b/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) diff --git a/templates/dashboard/setting.html b/templates/dashboard/setting.html index 00a781f0..ad4b7ee0 100644 --- a/templates/dashboard/setting.html +++ b/templates/dashboard/setting.html @@ -181,10 +181,10 @@
-
- - {{ change_email_form.csrf_token }} -
+
+ + + {{ change_email_form.csrf_token }}
Account Email
This email address is used to log in to SimpleLogin. @@ -199,26 +199,30 @@ {{ 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 %} - -
- Pending email change: {{ pending_email }} - - Resend - confirmation email - - - Cancel email - change - -
- {% endif %}
-
- + + {% if pending_email %} + +
+ Pending email change: {{ pending_email }} +
+ {{ change_email_form.csrf_token }} + Resend confirmation email +
+
+ {{ change_email_form.csrf_token }} + Cancel email change +
+
+ {% endif %} +
@@ -265,11 +269,15 @@
Password
-
You will receive an email containing instructions on how to change your password.
+
+ You will receive an email containing instructions on how to change your password. +
{{ csrf_form.csrf_token }} - +
diff --git a/tests/auth/test_reset_password.py b/tests/auth/test_reset_password.py new file mode 100644 index 00000000..20cd695b --- /dev/null +++ b/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