Page MenuHome

Move customer IP validation from charge() to the view
ClosedPublic

Authored by Anna Sirota (railla) on Mar 25 2021, 4:40 PM.

Details

Summary

Goal

The goal of this patch is to decouple IP address validation from the actual process of transacting a sale.
Right now, if an invalid IP address is passed via the request headers, the checkout views will fail with a DataError in the Transaction.charge() call, while still creating a Transaction. It'd be better if they instead failed earlier with a ValidationError.

This is also necessary to simplify refactoring of Transaction.charge() method and checkout views, which will have to happen later due to the ongoing issue with some recurring transactions caused by SCA.

What it does

  • adds a utility for validating IP addresses using Django builtin validators;
  • modifies checkout views to use this validation helper before proceeding to handling the form (similarly to how reCAPTCHA is handled);
  • adds a few tests that check that this validation actually works;
  • modifies the currency middleware which turns out to be breaking when passed a bogus IP address.

Diff Detail

Repository
rL Looper

Event Timeline

Anna Sirota (railla) requested review of this revision.Mar 25 2021, 4:40 PM
Anna Sirota (railla) created this revision.
Anna Sirota (railla) edited the summary of this revision. (Show Details)Mar 25 2021, 4:47 PM

LGTM. Two very small notes, no need to re-review>

looper/tests/test_checkout.py
530–535

activated_subs is not actually used in this test, so unless I'm missing something, this inner function and the initialisation of activated_subs can be removed.

looper/views/checkout.py
193

I'd add an explicit return None here, so that each code path has its explicit return value.

This revision is now accepted and ready to land.Mar 25 2021, 5:12 PM

Landed in rL3a15bc8e3202: Move customer IP validation to the views D10820

(Not yet used by devfund, which requires this change: D10821: Move customer IP validation from charge() to the views and its looper depencency updated by poetry)