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.
This commit is contained in:
Baptiste Jonglez 2024-03-29 19:43:56 +01:00 committed by Alexis Métaireau
parent 967266f7dc
commit a5bbaa61f7
No known key found for this signature in database
GPG key ID: 1C21B876828E5FF2
4 changed files with 67 additions and 20 deletions

View file

@ -14,6 +14,8 @@ from wtforms.fields import (
BooleanField, BooleanField,
DateField, DateField,
DecimalField, DecimalField,
HiddenField,
IntegerField,
Label, Label,
PasswordField, PasswordField,
SelectField, SelectField,
@ -437,6 +439,22 @@ class BillForm(FlaskForm):
raise ValidationError(msg) 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): class MemberForm(FlaskForm):
name = StringField(_("Name"), validators=[DataRequired()], filters=[strip_filter]) name = StringField(_("Name"), validators=[DataRequired()], filters=[strip_filter])

View file

@ -18,14 +18,24 @@
<td>{{ transaction.amount|currency }}</td> <td>{{ transaction.amount|currency }}</td>
<td> <td>
<span id="settle-bill" class="ml-auto pb-2"> <span id="settle-bill" class="ml-auto pb-2">
<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") }}">
<div
data-toggle="tooltip"
title='{{ _("Click here to record that the money transfer has been done") }}'
>
{{ _("Settle") }}
</div>
</button>
</form>
<a <a
href="{{ url_for('.add_settlement_bill', amount = transaction.amount, sender_id = transaction.ower.id, receiver_id = transaction.receiver.id) }}" href="{{ url_for('.add_settlement_bill', amount = transaction.amount, sender_id = transaction.ower.id, receiver_id = transaction.receiver.id) }}"
class="btn btn-primary" class="btn btn-primary"
> >
<div
data-toggle="tooltip"
title='{{ _("Click here to record that the money transfer has been done") }}'
>
{{ ("Settle") }} {{ ("Settle") }}
</div> </div>
</a> </a>

View file

@ -1358,23 +1358,25 @@ class TestBudget(IhatemoneyTestCase):
count = 0 count = 0
for t in transactions: for t in transactions:
count += 1 count += 1
self.client.get( self.client.post(
"/raclette/settle" "/raclette/settle",
+ "/" data={
+ str(t["amount"]) "amount": t["amount"],
+ "/" "sender_id": t["ower"].id,
+ str(t["ower"].id) "receiver_id": t["receiver"].id,
+ "/" },
+ str(t["receiver"].id)
) )
temp_transactions = project.get_transactions_to_settle_bill() temp_transactions = project.get_transactions_to_settle_bill()
# test if the one has disappeared # test if the one has disappeared
assert len(temp_transactions) == len(transactions) - count 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() bill = project.get_newest_bill()
assert bill.bill_type == models.BillType.REIMBURSEMENT 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): def test_settle_zero(self):
self.post_project("raclette") self.post_project("raclette")

View file

@ -56,6 +56,7 @@ from ihatemoney.forms import (
ProjectForm, ProjectForm,
ProjectFormWithCaptcha, ProjectFormWithCaptcha,
ResetPasswordForm, ResetPasswordForm,
SettlementForm,
get_billform_for, get_billform_for,
) )
from ihatemoney.history import get_history, get_history_queries, purge_history from ihatemoney.history import get_history, get_history_queries, purge_history
@ -853,16 +854,32 @@ def change_lang(lang):
def settle_bill(): def settle_bill():
"""Compute the sum each one have to pay to each other and display it""" """Compute the sum each one have to pay to each other and display it"""
transactions = g.project.get_transactions_to_settle_bill() transactions = g.project.get_transactions_to_settle_bill()
return render_template("settle_bills.html", transactions=transactions, current_view="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", 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"))
@main.route("/<project_id>/settle/<amount>/<int:sender_id>/<int:receiver_id>")
def add_settlement_bill(amount, sender_id, receiver_id):
settlement = Bill( settlement = Bill(
amount=float(amount), amount=form.amount.data,
date=datetime.datetime.today(), date=datetime.datetime.today(),
owers=[Person.query.get(receiver_id)], owers=[Person.query.get(form.receiver_id.data)],
payer_id=sender_id, payer_id=form.sender_id.data,
project_default_currency=g.project.default_currency, project_default_currency=g.project.default_currency,
bill_type=BillType.REIMBURSEMENT, bill_type=BillType.REIMBURSEMENT,
what=_("Settlement"), what=_("Settlement"),