From e7ab3c1a9545cde155b0e2d5ae7070c568c3e78b Mon Sep 17 00:00:00 2001 From: Glandos Date: Sun, 28 Aug 2022 15:11:23 +0200 Subject: [PATCH 01/10] Implement rate limiting with Flask-Limiter Fixes #1054 --- ihatemoney/run.py | 4 +++ ihatemoney/templates/layout.html | 6 ++++ ihatemoney/utils.py | 47 ++++--------------------- ihatemoney/web.py | 60 ++++++++++++++++---------------- setup.cfg | 1 + 5 files changed, 48 insertions(+), 70 deletions(-) diff --git a/ihatemoney/run.py b/ihatemoney/run.py index 69b9b32f..88f49463 100644 --- a/ihatemoney/run.py +++ b/ihatemoney/run.py @@ -1,3 +1,4 @@ +from datetime import datetime import os import os.path import warnings @@ -21,6 +22,7 @@ from ihatemoney.utils import ( IhmJSONEncoder, PrefixedWSGI, em_surround, + limiter, locale_from_iso, localize_list, minimal_round, @@ -170,6 +172,7 @@ def create_app( app.register_blueprint(web_interface) app.register_blueprint(apiv1) app.register_error_handler(404, page_not_found) + limiter.init_app(app) # Configure the a, root="main"pplication setup_database(app) @@ -187,6 +190,7 @@ def create_app( app.jinja_env.filters["minimal_round"] = minimal_round app.jinja_env.filters["em_surround"] = lambda text: Markup(em_surround(text)) app.jinja_env.filters["localize_list"] = localize_list + app.jinja_env.filters["from_timestamp"] = datetime.fromtimestamp # Translations and time zone (used to display dates). The timezone is # taken from the BABEL_DEFAULT_TIMEZONE settings, and falls back to diff --git a/ihatemoney/templates/layout.html b/ihatemoney/templates/layout.html index 67819adb..9a4ca0f0 100644 --- a/ihatemoney/templates/layout.html +++ b/ihatemoney/templates/layout.html @@ -134,6 +134,12 @@
{% block body %}
+ {% if breached_limit %} +

+ {{ limit_message }}
+ {{ _("Please retry after %(date)s.", date=breached_limit.reset_at | from_timestamp | datetimeformat )}} +

+ {% endif %} {% block content %}{% endblock %}
{% endblock %} diff --git a/ihatemoney/utils.py b/ihatemoney/utils.py index 513d35ea..ea2e3ac9 100644 --- a/ihatemoney/utils.py +++ b/ihatemoney/utils.py @@ -1,6 +1,5 @@ import ast import csv -from datetime import datetime, timedelta import email.utils from enum import Enum from io import BytesIO, StringIO, TextIOWrapper @@ -15,11 +14,18 @@ from babel import Locale from babel.numbers import get_currency_name, get_currency_symbol from flask import current_app, flash, redirect, render_template from flask_babel import get_locale, lazy_gettext as _ +from flask_limiter import Limiter +from flask_limiter.util import get_remote_address import jinja2 from markupsafe import Markup, escape from werkzeug.exceptions import HTTPException from werkzeug.routing import RoutingException +limiter = limiter = Limiter( + current_app, + key_func=get_remote_address, +) + def slugify(value): """Normalizes string, converts to lowercase, removes non-alpha characters, @@ -213,45 +219,6 @@ def csv2list_of_dicts(csv_to_convert): return result -class LoginThrottler: - """Simple login throttler used to limit authentication attempts based on client's ip address. - When using multiple workers, remaining number of attempts can get inconsistent - but will still be limited to num_workers * max_attempts. - """ - - def __init__(self, max_attempts=3, delay=1): - self._max_attempts = max_attempts - # Delay in minutes before resetting the attempts counter - self._delay = delay - self._attempts = {} - - def get_remaining_attempts(self, ip): - return self._max_attempts - self._attempts.get(ip, [datetime.now(), 0])[1] - - def increment_attempts_counter(self, ip): - # Reset all attempt counters when they get hungry for memory - if len(self._attempts) > 10000: - self.__init__() - if self._attempts.get(ip) is None: - # Store first attempt date and number of attempts since - self._attempts[ip] = [datetime.now(), 0] - self._attempts.get(ip)[1] += 1 - - def is_login_allowed(self, ip): - if self._attempts.get(ip) is None: - return True - # When the delay is expired, reset the counter - if datetime.now() - self._attempts.get(ip)[0] > timedelta(minutes=self._delay): - self.reset(ip) - return True - if self._attempts.get(ip)[1] >= self._max_attempts: - return False - return True - - def reset(self, ip): - self._attempts.pop(ip, None) - - def create_jinja_env(folder, strict_rendering=False): """Creates and return a Jinja2 Environment object, used, to load the templates. diff --git a/ihatemoney/web.py b/ihatemoney/web.py index c2f19c06..af50aed2 100644 --- a/ihatemoney/web.py +++ b/ihatemoney/web.py @@ -19,6 +19,7 @@ from flask import ( current_app, flash, g, + make_response, redirect, render_template, request, @@ -56,11 +57,11 @@ from ihatemoney.forms import ( from ihatemoney.history import get_history, get_history_queries, purge_history from ihatemoney.models import Bill, LoggingMode, Person, Project, db from ihatemoney.utils import ( - LoginThrottler, Redirect303, csv2list_of_dicts, flash_email_error, format_form_errors, + limiter, list_of_dicts2csv, list_of_dicts2json, render_localized_template, @@ -69,8 +70,6 @@ from ihatemoney.utils import ( main = Blueprint("main", __name__) -login_throttler = LoginThrottler(max_attempts=3, delay=1) - def requires_admin(bypass=None): """Require admin permissions for @requires_admin decorated endpoints. @@ -161,7 +160,22 @@ def health(): return "OK" +def admin_limit(limit): + return make_response( + render_template( + "admin.html", + breached_limit=limit, + limit_message=_("Too many failed login attempts."), + ) + ) + + @main.route("/admin", methods=["GET", "POST"]) +@limiter.limit( + "3/minute", + on_breach=admin_limit, + methods=["POST"], +) def admin(): """Admin authentication. @@ -170,33 +184,19 @@ def admin(): form = AdminAuthenticationForm() goto = request.args.get("goto", url_for(".home")) is_admin_auth_enabled = bool(current_app.config["ADMIN_PASSWORD"]) - if request.method == "POST": - client_ip = request.remote_addr - if not login_throttler.is_login_allowed(client_ip): - msg = _("Too many failed login attempts, please retry later.") - form["admin_password"].errors = [msg] - return render_template( - "admin.html", - form=form, - admin_auth=True, - is_admin_auth_enabled=is_admin_auth_enabled, - ) - if form.validate(): - # Valid password - if check_password_hash( - current_app.config["ADMIN_PASSWORD"], form.admin_password.data - ): - session["is_admin"] = True - session.update() - login_throttler.reset(client_ip) - return redirect(goto) - # Invalid password - login_throttler.increment_attempts_counter(client_ip) - msg = _( - "This admin password is not the right one. Only %(num)d attempts left.", - num=login_throttler.get_remaining_attempts(client_ip), - ) - form["admin_password"].errors = [msg] + if request.method == "POST" and form.validate(): + # Valid password + if check_password_hash( + current_app.config["ADMIN_PASSWORD"], form.admin_password.data + ): + session["is_admin"] = True + session.update() + return redirect(goto) + msg = _( + "This admin password is not the right one. Only %(num)d attempts left.", + num=limiter.current_limit.remaining, + ) + form["admin_password"].errors = [msg] return render_template( "admin.html", form=form, diff --git a/setup.cfg b/setup.cfg index 73ebcb08..01decaf9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -30,6 +30,7 @@ install_requires = email_validator>=1.0,<2 Flask-Babel>=1.0,<3 Flask-Cors>=3.0.8,<4 + Flask-Limiter>=2.6,<3 Flask-Mail>=0.9.1,<1 Flask-Migrate>=2.5.3,<4 # Not following semantic versioning (e.g. https://github.com/miguelgrinberg/flask-migrate/commit/1af28ba273de6c88544623b8dc02dd539340294b) Flask-RESTful>=0.3.9,<1 From 63fba6be4c3e601d7de98e1f714f025bb4c50281 Mon Sep 17 00:00:00 2001 From: Glandos Date: Sun, 28 Aug 2022 15:32:25 +0200 Subject: [PATCH 02/10] force in memory storage to remove warning we don't need persistent storage for now --- ihatemoney/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ihatemoney/utils.py b/ihatemoney/utils.py index ea2e3ac9..491eaf07 100644 --- a/ihatemoney/utils.py +++ b/ihatemoney/utils.py @@ -24,6 +24,7 @@ from werkzeug.routing import RoutingException limiter = limiter = Limiter( current_app, key_func=get_remote_address, + storage_uri="memory://", ) From d834394a44c6b5d2a7c98b148457bc73736abb91 Mon Sep 17 00:00:00 2001 From: Glandos Date: Sun, 28 Aug 2022 15:32:47 +0200 Subject: [PATCH 03/10] ensure current_limit exist before displaying any message based on it --- ihatemoney/web.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ihatemoney/web.py b/ihatemoney/web.py index af50aed2..b94a66d7 100644 --- a/ihatemoney/web.py +++ b/ihatemoney/web.py @@ -192,11 +192,13 @@ def admin(): session["is_admin"] = True session.update() return redirect(goto) - msg = _( - "This admin password is not the right one. Only %(num)d attempts left.", - num=limiter.current_limit.remaining, - ) - form["admin_password"].errors = [msg] + if limiter.current_limit is not None: + msg = _( + "This admin password is not the right one. Only %(num)d attempts left.", + # If the limiter is disabled, there is no current limit + num=limiter.current_limit.remaining, + ) + form["admin_password"].errors = [msg] return render_template( "admin.html", form=form, From 4d7e966f7f6eb292fe2f154ea05fcb5877a5be3e Mon Sep 17 00:00:00 2001 From: Glandos Date: Sun, 28 Aug 2022 15:34:17 +0200 Subject: [PATCH 04/10] fix tests --- ihatemoney/tests/budget_test.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/ihatemoney/tests/budget_test.py b/ihatemoney/tests/budget_test.py index 3214c173..2e33b0a5 100644 --- a/ihatemoney/tests/budget_test.py +++ b/ihatemoney/tests/budget_test.py @@ -601,22 +601,23 @@ class BudgetTestCase(IhatemoneyTestCase): ) self.assertIn( - "Too many failed login attempts, please retry later.", + "Too many failed login attempts.", resp.data.decode("utf-8"), ) - # Change throttling delay - from ihatemoney.web import login_throttler + # Try with limiter disabled + from ihatemoney.utils import limiter - login_throttler._delay = 0.005 - # Wait for delay to expire and retry logging in - sleep(1) - resp = self.client.post( - "/admin?goto=%2Fcreate", data={"admin_password": "wrong"} - ) - self.assertNotIn( - "Too many failed login attempts, please retry later.", - resp.data.decode("utf-8"), - ) + try: + limiter.enabled = False + resp = self.client.post( + "/admin?goto=%2Fcreate", data={"admin_password": "wrong"} + ) + self.assertNotIn( + "Too many failed login attempts.", + resp.data.decode("utf-8"), + ) + finally: + limiter.enabled = True def test_manage_bills(self): self.post_project("raclette") From 75ed637911e02b251d66ae5afd3c855a96f29ad0 Mon Sep 17 00:00:00 2001 From: Glandos Date: Thu, 27 Oct 2022 22:15:32 +0200 Subject: [PATCH 05/10] short date, clearer output --- ihatemoney/templates/layout.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ihatemoney/templates/layout.html b/ihatemoney/templates/layout.html index 9a4ca0f0..4d8cf6d6 100644 --- a/ihatemoney/templates/layout.html +++ b/ihatemoney/templates/layout.html @@ -137,7 +137,7 @@ {% if breached_limit %}

{{ limit_message }}
- {{ _("Please retry after %(date)s.", date=breached_limit.reset_at | from_timestamp | datetimeformat )}} + {{ _("Please retry after %(date)s.", date=breached_limit.reset_at | from_timestamp | datetimeformat("short") )}}

{% endif %} {% block content %}{% endblock %} From 19f5f07bfb325759f8576deb52cc84015fc550be Mon Sep 17 00:00:00 2001 From: Glandos Date: Thu, 27 Oct 2022 22:15:48 +0200 Subject: [PATCH 06/10] don't display content block on rate limit --- ihatemoney/templates/layout.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ihatemoney/templates/layout.html b/ihatemoney/templates/layout.html index 4d8cf6d6..c99960eb 100644 --- a/ihatemoney/templates/layout.html +++ b/ihatemoney/templates/layout.html @@ -139,8 +139,9 @@ {{ limit_message }}
{{ _("Please retry after %(date)s.", date=breached_limit.reset_at | from_timestamp | datetimeformat("short") )}}

- {% endif %} + {% else %} {% block content %}{% endblock %} + {% endif %} {% endblock %}
From 9ccfc2981dfcefb66f0fecf9364df9f4035bd9f1 Mon Sep 17 00:00:00 2001 From: Glandos Date: Thu, 27 Oct 2022 22:26:13 +0200 Subject: [PATCH 07/10] side-effect: add autofocus on admin field --- ihatemoney/forms.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ihatemoney/forms.py b/ihatemoney/forms.py index c4fc32a8..e9973fdd 100644 --- a/ihatemoney/forms.py +++ b/ihatemoney/forms.py @@ -295,7 +295,9 @@ class AuthenticationForm(FlaskForm): class AdminAuthenticationForm(FlaskForm): - admin_password = PasswordField(_("Admin password"), validators=[DataRequired()]) + admin_password = PasswordField( + _("Admin password"), validators=[DataRequired()], render_kw={"autofocus": True} + ) submit = SubmitField(_("Get in")) From 9a8cc16a0b4ae5082858cafad17107ed8d2fe00d Mon Sep 17 00:00:00 2001 From: Glandos Date: Thu, 27 Oct 2022 22:26:22 +0200 Subject: [PATCH 08/10] unused import --- ihatemoney/tests/budget_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ihatemoney/tests/budget_test.py b/ihatemoney/tests/budget_test.py index 2e33b0a5..fb434fbb 100644 --- a/ihatemoney/tests/budget_test.py +++ b/ihatemoney/tests/budget_test.py @@ -1,7 +1,6 @@ from collections import defaultdict import datetime import re -from time import sleep import unittest from urllib.parse import urlparse, urlunparse From 7714ed91986034d4204d8189bb23cfb0720e3181 Mon Sep 17 00:00:00 2001 From: Alexis Metaireau Date: Sun, 11 Dec 2022 12:44:47 +0100 Subject: [PATCH 09/10] Favicon update. (#1102) Original work by @asya99 in #1024. Thanks for this :-) --- ihatemoney/static/favicon.ico | Bin 1742 -> 6742 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/ihatemoney/static/favicon.ico b/ihatemoney/static/favicon.ico index 9db04f25d60d2ab20a6d3a83b8bf3a666b7fa5ee..070642e34160afc6132f1c5b0123fac028c4e5d7 100644 GIT binary patch literal 6742 zcmeI0YmAiD8OP5oHy6;w3kpV=#nQzT6{}Kg6-L2Z(h#qp(kL3i3&v7o=@-$(!Ur0w z)EJ3rtrY_l=AGFE4QNCoTN>AI8jW`itE1st17GY^w{!gc&%@bwW_E^M+BCtWC;88F z-gBPYdCqfg<6OqIxJi>7wB4<1b*{rX*WPa5TPHZzMcbLQ!Q##Z;tPfLy~i1>;VAmu z>Ydx_Gj5w7<%+%qIQLiZpO$UNCZOQQKz{^X0KEu21~!6~;C$LgL66Le>1_$}iC+O- zik?fs6xZcv^L;aT6ubsDfDXpR#0iw5 z7bqe#p`o1oF8UGZR!n{fQjMFb%T~dk1kX&jrK(R{=2t8FH&eeYjG=xmjAttSccUXS z;Dfb~ru{0gf%YPFO3P?2qm{m<#!#&_$0y!xfX`rD~b3*Ty2G5yCw7bCM6+g;RG zgETKCoA)Qurkr>KoCjJ4Y?zL7Y5Nmxr$SA4^{;#^`m@QwC+Pn@7&pLY^5@}?`$bLH8R&Q$EC;d8xt&3NB0M|6jDfgCd6S(> zUmpDr(SHgUS;Hrue8`KLwAbdS#@EgG6#v983AU@(h{aHAdIhq7gWeBXLG&Qz?^yb; z0HZ+KXY;S|EChLw&Ryh^u7W?EIlhK5j1E4;{ET~H;x9&aaaeEF`lZ8s)Est$C4+dh z7Ym9({l0ARQ-1y$Oa{@bl*$*IletjEVD2CrDR){k-O4|$`-_5(`n4Om#cMWhI%H!Z z;~HHP%g8G}joJ3lUqUahS@)5zo|^s)`maRhW$+iEb$^=M;m5mO?0ZF^bzbm`pbvl_ zQ+La= zO8Z^upS|#|cjeADS9V!fL63g~ebNhb4{HaPfPs4$ceJDS9)_RatD-fP4`++cy~*V6 zSoY!!`TM^ahV4h%%R8{8{YC404$!{&5x(q!>W(=djN{umt+W5OXpQS$ocNz$b1n37 z=v`nQn9P{97yN?uH9+^l67(f-8FIB}*^&B^?!ot=?{RQ(*y9vq-P>c)d9SlUb6{u9 z7X0{%`f5;nrbn5fsLk&Le==lCu5K?*6yp!#?mda>orzHfpzV>+ZLTF)bsf zMnGfl&{Xl*1e6PT|0!69e0r}toCd{KV^91IhwKkeu3o2f*!gfB`o0MIbhZqYlQ#BA z@aw*JI;hud-^MqB{;}>Ecr@PnBAx=V%ZJ=~gZjkaQ$1h&j^~->^Ri&Ok$uI;&dKx1 z$A{f+Z0u@aUwX~Y4t)5K`fVY$l5JFsS8t<7@l)%(mP;|!{Z8k4!ki}lb?^yT7}jV# zzog~sj3A?UzJSirgY-r#W?y%_YvCFEMw9(*VK2~nybqq6vD-sk>-}fIxs@Om@#n+a z=>+=p?)w;-GaA}A+v2;O{u3JVXpQTQr8}?2THwym9oE-F*jX_swr2kn+Ft=$i(jK( z_ohYE#{%;-?k5RrE$_?puO)6H8Cx7lEmgbOpAFA0__YUL4O~qg=v(=xeOYgq6q>i= zgMHH@KD~pqkBjdh{R@IV>8LKUEx!{#3&?gK&|EZr_gY-1BJ=Jc-T#39%AiBzmi;$_ zzTeRP4(*DO-bud-ef9QrN8CXU=}naQKM0wv;NaT_NXR zhHqUsUmEf0+)=yUGYN0LqJNR^E2#f8jI;V&irU4mGqjttNOy!rc_kmUFC^?I32)f~ zXGSCY(kc1wkXLuZ^C|p#n_UiS&&6u{1wS3$<;XUF57@Y7F^+xc(%Ztu8ue6D@fLVX z7kwvh>~Ze7CcNUa`M)22ohddS)w*Kc?CHFD9vS6!^K(-=wYO*;KTBQz#nczgh1oj` zs{bE!p4W4w-#h3tU-f^}XYf8iu4fJ_lS&V+ne8^@A7kv7fadXx(wf_Qf&R;~|0Qm6 zFXDq_kEEC_?d1=GrN|F|9Ok#y_;2yyE9gByZ_A-QkX@748LqpN{*!Bd<}}kN9op0K z{O@8uY(dv7^3DFE&^%R__;xLZ|0tBZPk}`s-D|4(hf}w4X#buMbcTEZx~MB2bIAJ{ z%*}N6jbrik9NO-Hu7|Dy(?d?y`#BuD)jW!)bZ9@YGiD>-?@;dp`)L0dx&V3y^cFA; ze~#?FSZ(icb;ZN}3!R9anfQ?d^QfO3d_0`&|ND;02=HHll}elUeIq%LDf-5O8F zyL@HO`8w@NoxTl1d*EMq`TQU>d*pq?=`S7LgLo%+XQeV1N)-lR@X&0APZQK~YyYOl zGH>SVCUAc6+UdS3T`x?Ih)%ktN?yb{4sgCM;=FIDWK2fs)uL>y^L-fjBjoI6QO0{; zn`dP{at+GjM2>F2hsXFnOKqoykUvTcr)o&!br_lEC5(WY{b_)&}N zRq{aFCOBss%5A99$>WIU^hC?yMXMe%G8e)5UbcU2uhc@$xVGHSl{z_r-|inoJ>O-0 ztn3-SuUuPxBLZ!Xa;{gD7v^@Z#`h1kZSh;hJRz;0AvN_0Emu(hC{%|nQZwI{#r2qf` From 4bef5ad9224a6da876415674625d7de30897c0b0 Mon Sep 17 00:00:00 2001 From: cbrosnan <55763924+cbrosnan@users.noreply.github.com> Date: Sun, 11 Dec 2022 15:19:31 -0500 Subject: [PATCH 10/10] added confirmation for expense deletion (#1096) --- ihatemoney/static/css/main.css | 5 +++++ ihatemoney/templates/list_bills.html | 23 ++++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ihatemoney/static/css/main.css b/ihatemoney/static/css/main.css index 4c61fbea..e0d8e86e 100644 --- a/ihatemoney/static/css/main.css +++ b/ihatemoney/static/css/main.css @@ -310,6 +310,11 @@ footer .footer-left { background: url("../images/delete.png") no-repeat right; } +.bill-actions > form > .confirm.btn-danger { + outline-color: #c82333; + box-shadow: none; +} + .bill-actions > .edit { background: url("../images/edit.png") no-repeat right; } diff --git a/ihatemoney/templates/list_bills.html b/ihatemoney/templates/list_bills.html index 0445d99d..27c0e291 100644 --- a/ihatemoney/templates/list_bills.html +++ b/ihatemoney/templates/list_bills.html @@ -39,8 +39,29 @@ $('#bill_table tbody tr').hover(highlight_owers, unhighlight_owers); + + + let link = $('#delete-bill').find('button'); + let deleteOriginalHTML = link.html(); + link.click(function() { + if (link.hasClass("confirm")){ + return true; + } + link.html("{{_("Are you sure?")}}"); + link.removeClass("action delete"); + link.addClass("confirm btn-danger"); + return false; + }); + + $('#delete-bill').focusout(function() { + link.removeClass("confirm btn-danger"); + link.html(deleteOriginalHTML); + link.addClass("action delete"); + }); + {% endblock %} + {% block sidebar %}
@@ -137,7 +158,7 @@ {{ _('edit') }} - + {{ csrf_form.csrf_token }}