Merge 01d515c07f into 7505cbe25a

feat: Add a SITE_NAME setting and use it everywhere.
Add security-related test to the new settle endpoint
2025-05-14 08:21:49 +02:00 · 2024-12-20 23:27:16 +01:00 · 2024-12-20 23:27:11 +01:00 · 2024-03-31 19:55:43 +02:00 · 2024-03-31 19:55:43 +02:00 · 2024-03-31 19:55:43 +02:00
9 changed files with 182 additions and 29 deletions
--- a/docs/configuration.md
+++ b/docs/configuration.md
@ -173,6 +173,14 @@ URL you want.
 -   **Default value:** `""` (empty string)
 -   **Production value:** The URL of your chosing.

+## SITE_NAME
+
+It is possible to change the name of the site to something at your liking.
+
+-   **Default value:** `"I Hate Money"` (empty string)
+-   **Production value:** The name of your choosing
+
+
 ## Configuring email sending

 By default, Ihatemoney sends emails using a local SMTP server, but it's
--- a/ihatemoney/default_settings.py
+++ b/ihatemoney/default_settings.py
@ -3,6 +3,7 @@ DEBUG = SQLACHEMY_ECHO = False
 SQLALCHEMY_DATABASE_URI = "sqlite:////tmp/ihatemoney.db"
 SQLALCHEMY_TRACK_MODIFICATIONS = False
 SECRET_KEY = "tralala"
+SITE_NAME = "I Hate Money"
 MAIL_DEFAULT_SENDER = "Budget manager <admin@example.com>"
 SHOW_ADMIN_EMAIL = True
 ACTIVATE_DEMO_PROJECT = True
--- a/ihatemoney/forms.py
+++ b/ihatemoney/forms.py
@ -14,6 +14,8 @@ from wtforms.fields import (
    BooleanField,
    DateField,
    DecimalField,
+    HiddenField,
+    IntegerField,
    Label,
    PasswordField,
    SelectField,
@ -437,6 +439,22 @@ class BillForm(FlaskForm):
            raise ValidationError(msg)


+class HiddenCommaDecimalField(HiddenField, CommaDecimalField):
+    pass
+
+
+class HiddenIntegerField(HiddenField, IntegerField):
+    pass
+
+
+class SettlementForm(FlaskForm):
+    """Used internally for validation, not directly visible to users"""
+
+    amount = HiddenCommaDecimalField("Amount", validators=[DataRequired()])
+    sender_id = HiddenIntegerField("Sender", validators=[DataRequired()])
+    receiver_id = HiddenIntegerField("Receiver", validators=[DataRequired()])
+
+
 class MemberForm(FlaskForm):
    name = StringField(_("Name"), validators=[DataRequired()], filters=[strip_filter])

--- a/ihatemoney/templates/layout.html
+++ b/ihatemoney/templates/layout.html
@ -20,7 +20,7 @@
 <!DOCTYPE html>
 <html class="h-100">
 <head>
-    <title>{{ _("Account manager") }}{% block title %}{% endblock %}</title>
+    <title>{{ SITE_NAME }} — {{ _("Account manager") }}{% block title %}{% endblock %}</title>
    <meta http-equiv="content-type" content="text/html; charset=utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <link rel=stylesheet type=text/css href="{{ url_for("static", filename='css/main.css') }}">
--- a/ihatemoney/templates/settle_bills.html
+++ b/ihatemoney/templates/settle_bills.html
@ -11,15 +11,20 @@
    <table id="bill_table" class="split_bills table table-striped">
        <thead><tr><th>{{ _("Who pays?") }}</th><th>{{ _("To whom?") }}</th><th>{{ _("How much?") }}</th><th>{{ _("Settled?") }}</th></tr></thead>
    <tbody>
-    {% for bill in bills %}
-    <tr receiver={{bill.receiver.id}}>
-            <td>{{ bill.ower }}</td>
-            <td>{{ bill.receiver }}</td>
-            <td>{{ bill.amount|currency }}</td>
+    {% for transaction in transactions %}
+    <tr>
+            <td>{{ transaction.ower }}</td>
+            <td>{{ transaction.receiver }}</td>
+            <td>{{ transaction.amount|currency }}</td>
            <td>
                <span id="settle-bill" class="ml-auto pb-2">
-                    <a href="{{ url_for('.settle', amount = bill.amount, ower_id = bill.ower.id, payer_id = bill.receiver.id) }}" class="btn btn-primary">
-                        {{ ("Settle") }}
+                    <form class="" action="{{ url_for(".add_settlement_bill") }}" method="POST">
+                        {{ settlement_form.csrf_token }}
+                        {{ settlement_form.amount(value=transaction.amount) }}
+                        {{ settlement_form.sender_id(value=transaction.ower.id) }}
+                        {{ settlement_form.receiver_id(value=transaction.receiver.id) }}
+                        <button class="btn btn-primary" type="submit" title="{{ _("Settle") }}">{{ _("Settle") }}</button>
+                    </form>
                    </a>
                </span>
            </td>
--- a/ihatemoney/tests/budget_test.py
+++ b/ihatemoney/tests/budget_test.py
@ -238,7 +238,10 @@ class TestBudget(IhatemoneyTestCase):
            url, data={"password": "pass", "password_confirmation": "pass"}
        )
        resp = self.login("raclette", password="pass")
-        assert "<title>Account manager - raclette</title>" in resp.data.decode("utf-8")
+        assert (
+            "<title>I Hate Money — Account manager - raclette</title>"
+            in resp.data.decode("utf-8")
+        )
        # Test empty and null tokens
        resp = self.client.get("/reset-password")
        assert "No token provided" in resp.data.decode("utf-8")
@ -1355,23 +1358,25 @@ class TestBudget(IhatemoneyTestCase):
        count = 0
        for t in transactions:
            count += 1
-            self.client.get(
-                "/raclette/settle"
-                + "/"
-                + str(t["amount"])
-                + "/"
-                + str(t["ower"].id)
-                + "/"
-                + str(t["receiver"].id)
+            self.client.post(
+                "/raclette/settle",
+                data={
+                    "amount": t["amount"],
+                    "sender_id": t["ower"].id,
+                    "receiver_id": t["receiver"].id,
+                },
            )
            temp_transactions = project.get_transactions_to_settle_bill()
            # test if the one has disappeared
            assert len(temp_transactions) == len(transactions) - count

-            # test if theres a new one with bill_type reimbursement
+            # test if there is a new one with bill_type reimbursement
            bill = project.get_newest_bill()
            assert bill.bill_type == models.BillType.REIMBURSEMENT
-        return
+
+        # There should be no more settlement to do at the end
+        transactions = project.get_transactions_to_settle_bill()
+        assert len(transactions) == 0

    def test_settle_zero(self):
        self.post_project("raclette")
@ -1460,6 +1465,78 @@ class TestBudget(IhatemoneyTestCase):
        # Create and log in as another project
        self.post_project("tartiflette")

+        # Add a participant in this second project
+        self.client.post("/tartiflette/members/add", data={"name": "pirate"})
+        pirate = models.Person.query.filter(models.Person.id == 5).one()
+        assert pirate.name == "pirate"
+
+        # Try to add a new bill in another project
+        self.client.post(
+            "/raclette/add",
+            data={
+                "date": "2017-01-01",
+                "what": "fromage frelaté",
+                "payer": 2,
+                "payed_for": [2, 3, 4],
+                "bill_type": "Expense",
+                "amount": "100.0",
+            },
+        )
+        # Ensure it has not been created
+        raclette = self.get_project("raclette")
+        assert raclette.get_bills().count() == 1
+
+        # Try to add a new bill in our project that references members of another project.
+        # First with invalid payed_for IDs.
+        self.client.post(
+            "/tartiflette/add",
+            data={
+                "date": "2017-01-01",
+                "what": "soupe",
+                "payer": 5,
+                "payed_for": [3],
+                "bill_type": "Expense",
+                "amount": "5000.0",
+            },
+        )
+        # Ensure it has not been created
+        piratebill = models.Bill.query.filter(models.Bill.what == "soupe").one_or_none()
+        assert piratebill is None, "piratebill 1 should not exist"
+
+        # Then with invalid payer ID
+        self.client.post(
+            "/tartiflette/add",
+            data={
+                "date": "2017-02-01",
+                "what": "pain",
+                "payer": 3,
+                "payed_for": [5],
+                "bill_type": "Expense",
+                "amount": "5000.0",
+            },
+        )
+        # Ensure it has not been created
+        piratebill = models.Bill.query.filter(models.Bill.what == "pain").one_or_none()
+        assert piratebill is None, "piratebill 2 should not exist"
+
+        # Make sure we can actually create valid bills
+        self.client.post(
+            "/tartiflette/add",
+            data={
+                "date": "2017-03-01",
+                "what": "baguette",
+                "payer": 5,
+                "payed_for": [5],
+                "bill_type": "Expense",
+                "amount": "5.0",
+            },
+        )
+        # Ensure it has been created
+        okbill = models.Bill.query.filter(models.Bill.what == "baguette").one_or_none()
+        assert okbill is not None, "Bill baguette should exist"
+        assert okbill.what == "baguette"
+
+        # Now try to access and modify existing bills
        modified_bill = {
            "date": "2018-12-31",
            "what": "roblochon",
@ -1553,6 +1630,24 @@ class TestBudget(IhatemoneyTestCase):
        member = models.Person.query.filter(models.Person.id == 1).one_or_none()
        assert member is None

+        # test new settle endpoint to add bills with wrong payer / payed_for
+        self.client.post("/exit")
+        self.client.post(
+            "/authenticate", data={"id": "tartiflette", "password": "tartiflette"}
+        )
+        self.client.post(
+            "/tartiflette/settle",
+            data={
+                "sender_id": 4,
+                "receiver_id": 5,
+                "amount": "42.0",
+            },
+        )
+        piratebill = models.Bill.query.filter(
+            models.Bill.bill_type == models.BillType.REIMBURSEMENT
+        ).one_or_none()
+        assert piratebill is None, "piratebill 3 should not exist"
+
    @pytest.mark.skip(reason="Currency conversion is broken")
    def test_currency_switch(self):
        # A project should be editable
--- a/ihatemoney/tests/ihatemoney.cfg
+++ b/ihatemoney/tests/ihatemoney.cfg
@ -3,6 +3,7 @@
 DEBUG = False
 SQLALCHEMY_DATABASE_URI = 'sqlite:///budget.db'
 SQLACHEMY_ECHO = DEBUG
+SITE_NAME = "I Hate Money"

 SECRET_KEY = "supersecret"

--- a/ihatemoney/utils.py
+++ b/ihatemoney/utils.py
@ -452,7 +452,9 @@ def format_form_errors(form, prefix):
        )
    else:
        error_list = "</li><li>".join(
-            str(error) for (field, errors) in form.errors.items() for error in errors
+            f"<strong>{field}</strong> {error}"
+            for (field, errors) in form.errors.items()
+            for error in errors
        )
        errors = f"<ul><li>{error_list}</li></ul>"
        # I18N: Form error with a list of errors
--- a/ihatemoney/web.py
+++ b/ihatemoney/web.py
@ -56,6 +56,7 @@ from ihatemoney.forms import (
    ProjectForm,
    ProjectFormWithCaptcha,
    ResetPasswordForm,
+    SettlementForm,
    get_billform_for,
 )
 from ihatemoney.history import get_history, get_history_queries, purge_history
@ -137,6 +138,11 @@ def set_show_admin_dashboard_link(endpoint, values):
    g.logout_form = LogoutForm()


+@main.context_processor
+def add_template_variables():
+    return {"SITE_NAME": current_app.config.get("SITE_NAME")}
+
+
@main.url_value_preprocessor
 def pull_project(endpoint, values):
    """When a request contains a project_id value, transform it directly
@ -847,24 +853,41 @@ def change_lang(lang):
@main.route("/<project_id>/settle_bills")
 def settle_bill():
    """Compute the sum each one have to pay to each other and display it"""
-    bills = g.project.get_transactions_to_settle_bill()
-    return render_template("settle_bills.html", bills=bills, current_view="settle_bill")
+    transactions = g.project.get_transactions_to_settle_bill()
+    settlement_form = SettlementForm()
+    return render_template(
+        "settle_bills.html",
+        transactions=transactions,
+        settlement_form=settlement_form,
+        current_view="settle_bill",
+    )


-@main.route("/<project_id>/settle/<amount>/<int:ower_id>/<int:payer_id>")
-def settle(amount, ower_id, payer_id):
-    new_reinbursement = Bill(
-        amount=float(amount),
+@main.route("/<project_id>/settle", methods=["POST"])
+def add_settlement_bill():
+    """Create a bill to register a settlement"""
+    form = SettlementForm(id=g.project.id)
+    if not form.validate():
+        flash(
+            format_form_errors(form, _("Error creating settlement bill")),
+            category="danger",
+        )
+        return redirect(url_for(".settle_bill"))
+
+    # TODO: check that sender and receiver ID are valid and part of this project
+
+    settlement = Bill(
+        amount=form.amount.data,
        date=datetime.datetime.today(),
-        owers=[Person.query.get(payer_id)],
-        payer_id=ower_id,
+        owers=[Person.query.get(form.receiver_id.data)],
+        payer_id=form.sender_id.data,
        project_default_currency=g.project.default_currency,
        bill_type=BillType.REIMBURSEMENT,
        what=_("Settlement"),
    )
    session.update()

-    db.session.add(new_reinbursement)
+    db.session.add(settlement)
    db.session.commit()

    flash(_("Settlement bill has been successfully added"), category="success")
Author	SHA1	Message	Date
zorun	6056d2ea06	Merge `01d515c07f` into `7505cbe25a`	2024-12-20 23:27:16 +01:00
Mickaël Schoentgen	7505cbe25a	feat: Add a `SITE_NAME` setting and use it everywhere. Some checks failed CI / lint (push) Has been cancelled Details CI / docs (push) Has been cancelled Details Docker build / test (push) Has been cancelled Details CI / test (mariadb, minimal, 3.11) (push) Has been cancelled Details CI / test (mariadb, normal, 3.11) (push) Has been cancelled Details CI / test (mariadb, normal, 3.9) (push) Has been cancelled Details CI / test (postgresql, minimal, 3.11) (push) Has been cancelled Details CI / test (postgresql, normal, 3.11) (push) Has been cancelled Details CI / test (postgresql, normal, 3.9) (push) Has been cancelled Details CI / test (sqlite, minimal, 3.10) (push) Has been cancelled Details CI / test (sqlite, minimal, 3.11) (push) Has been cancelled Details CI / test (sqlite, minimal, 3.12) (push) Has been cancelled Details CI / test (sqlite, minimal, 3.9) (push) Has been cancelled Details CI / test (sqlite, normal, 3.10) (push) Has been cancelled Details CI / test (sqlite, normal, 3.11) (push) Has been cancelled Details CI / test (sqlite, normal, 3.12) (push) Has been cancelled Details CI / test (sqlite, normal, 3.8) (push) Has been cancelled Details CI / test (sqlite, normal, 3.9) (push) Has been cancelled Details Docker build / build_upload (push) Has been cancelled Details	2024-12-20 23:27:11 +01:00
Baptiste Jonglez	01d515c07f	Add security-related test to the new settle endpoint	2024-03-31 19:55:43 +02:00
Baptiste Jonglez	9a848588c8	tests: add more validation against cross-project access	2024-03-31 19:55:43 +02:00
Baptiste Jonglez	278a976501	Change settle endpoint to use POST instead of GET This is necessary to avoid XSS. State-changing actions should never be done with a GET.	2024-03-31 19:55:43 +02:00
Baptiste Jonglez	b09b9c1629	utils: improve error message when form field validation fails	2024-03-31 19:55:43 +02:00
Baptiste Jonglez	9b01f69e5c	settle_bills: fix HTML	2024-03-31 19:55:43 +02:00
Baptiste Jonglez	d6291a97e6	Settlement: rename variables to make the code more understandable	2024-03-31 19:55:40 +02:00