From 1c34b2d601a8901cae032cd84353c81158137af0 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 24 May 2024 14:52:20 +0000 Subject: [PATCH 1/5] Make IP address optional when reporting transactions --- HISTORY.rst | 8 ++++++++ README.rst | 3 ++- minfraud/validation.py | 28 +++++++++++++++++++++++++--- tests/test_validation.py | 14 ++++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 4a6bc22..f707936 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -3,6 +3,14 @@ History ------- +2.11.0 ++++++++++++++++++++ + +* Updated the validation for the Report Transactions API to make the + ``ip_address`` parameter optional. Now the ``tag`` and at least one of the + following parameters must be supplied: ``ip_address``, ``maxmind_id``, + ``minfraud_id``, ``transaction_id``. + 2.10.0 (2024-04-16) +++++++++++++++++++ diff --git a/README.rst b/README.rst index e4b8bc2..144761d 100644 --- a/README.rst +++ b/README.rst @@ -98,7 +98,8 @@ The method takes a dictionary representing the report to be sent to the web service. The structure of this dictionary should be in `the format specified in the REST API documentation `__. The -``ip_address`` and ``tag`` fields are required. All other fields are optional. +required fields are ``tag`` and one or more of the following: ``ip_address``, +``maxmind_id``, ``minfraud_id``, ``transaction_id``. Request Validation (for all request methods) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/minfraud/validation.py b/minfraud/validation.py index 7116787..a741f88 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -379,14 +379,36 @@ def _uuid(s: str) -> str: raise ValueError -validate_report = Schema( +def _transaction_id(s: Optional[str]) -> str: + if isinstance(s, str) and len(s) > 0: + return s + raise ValueError + + +validate_report_schema = Schema( { "chargeback_code": str, - Required("ip_address"): _ip_address, + "ip_address": _ip_address, "maxmind_id": _maxmind_id, "minfraud_id": _uuid, "notes": str, Required("tag"): _tag, - "transaction_id": str, + "transaction_id": _transaction_id, }, ) + + +def validate_at_least_one_identifier_field(report): + optional_fields = ["ip_address", "maxmind_id", "minfraud_id", "transaction_id"] + if not any(field in report for field in optional_fields): + raise ValueError( + "The report must contain at least one of the following fields: 'ip_address', 'maxmind_id', 'minfraud_id', 'transaction_id'." + ) + return True + + +def validate_report(report): + """Validate minFraud Transaction Report fields.""" + validate_report_schema(report) + validate_at_least_one_identifier_field(report) + return True diff --git a/tests/test_validation.py b/tests/test_validation.py index 62a8953..2e12a34 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -51,11 +51,17 @@ def setup_report(self, report): def check_invalid_report(self, report): self.setup_report(report) + self.check_invalid_report_no_setup(report) + + def check_invalid_report_no_setup(self, report): with self.assertRaises(MultipleInvalid, msg=f"{report} is invalid"): validate_report(report) def check_report(self, report): self.setup_report(report) + self.check_report_no_setup(report) + + def check_report_no_setup(self, report): try: validate_report(report) except MultipleInvalid as e: @@ -431,3 +437,11 @@ def test_tag(self): self.check_report({"tag": good}) for bad in ("risky_business", "", None): self.check_invalid_report({"tag": bad}) + + def test_report_valid_identifier(self): + self.check_report_no_setup({"tag": "chargeback", "ip_address": "1.1.1.1"}) + self.check_report_no_setup( + {"tag": "chargeback", "minfraud_id": "58fa38d8-4b87-458b-a22b-f00eda1aa20d"} + ) + self.check_report_no_setup({"tag": "chargeback", "maxmind_id": "12345678"}) + self.check_report_no_setup({"tag": "chargeback", "transaction_id": "abc123"}) From 7c6c159001b4d93babe1814b25e395673dbc1ada Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Mon, 3 Jun 2024 17:20:04 +0000 Subject: [PATCH 2/5] Use internal naming convention --- minfraud/validation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index a741f88..5d7b60b 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -385,7 +385,7 @@ def _transaction_id(s: Optional[str]) -> str: raise ValueError -validate_report_schema = Schema( +_validate_report_schema = Schema( { "chargeback_code": str, "ip_address": _ip_address, @@ -398,7 +398,7 @@ def _transaction_id(s: Optional[str]) -> str: ) -def validate_at_least_one_identifier_field(report): +def _validate_at_least_one_identifier_field(report): optional_fields = ["ip_address", "maxmind_id", "minfraud_id", "transaction_id"] if not any(field in report for field in optional_fields): raise ValueError( @@ -409,6 +409,6 @@ def validate_at_least_one_identifier_field(report): def validate_report(report): """Validate minFraud Transaction Report fields.""" - validate_report_schema(report) - validate_at_least_one_identifier_field(report) + _validate_report_schema(report) + _validate_at_least_one_identifier_field(report) return True From cbef79e62ce0b839e37f96b21cdfdc28f019e5ba Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Thu, 6 Jun 2024 17:50:13 +0000 Subject: [PATCH 3/5] Return MultipleInvalid instead of ValueError --- minfraud/validation.py | 9 ++++++--- tests/test_validation.py | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index 5d7b60b..61e3937 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -15,7 +15,7 @@ from typing import Optional from email_validator import validate_email # type: ignore -from voluptuous import All, Any, In, Match, Range, Required, Schema +from voluptuous import All, Any, In, Match, MultipleInvalid, Range, Required, Schema from voluptuous.error import UrlInvalid # Pylint doesn't like the private function type naming for the callable @@ -401,8 +401,11 @@ def _transaction_id(s: Optional[str]) -> str: def _validate_at_least_one_identifier_field(report): optional_fields = ["ip_address", "maxmind_id", "minfraud_id", "transaction_id"] if not any(field in report for field in optional_fields): - raise ValueError( - "The report must contain at least one of the following fields: 'ip_address', 'maxmind_id', 'minfraud_id', 'transaction_id'." + # We return MultipleInvalid instead of ValueError to be consistent with what + # voluptuous returns. + raise MultipleInvalid( + "The report must contain at least one of the following fields: " + "'ip_address', 'maxmind_id', 'minfraud_id', 'transaction_id'." ) return True diff --git a/tests/test_validation.py b/tests/test_validation.py index 2e12a34..a57d9d6 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -439,6 +439,8 @@ def test_tag(self): self.check_invalid_report({"tag": bad}) def test_report_valid_identifier(self): + self.check_invalid_report_no_setup({"tag": "chargeback"}) + self.check_report_no_setup({"tag": "chargeback", "ip_address": "1.1.1.1"}) self.check_report_no_setup( {"tag": "chargeback", "minfraud_id": "58fa38d8-4b87-458b-a22b-f00eda1aa20d"} From aabb36f17b72fae78b4bbc146e218884b400d188 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Thu, 6 Jun 2024 20:37:56 +0000 Subject: [PATCH 4/5] Simplify expression --- minfraud/validation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index 61e3937..fe9c0d3 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -399,8 +399,8 @@ def _transaction_id(s: Optional[str]) -> str: def _validate_at_least_one_identifier_field(report): - optional_fields = ["ip_address", "maxmind_id", "minfraud_id", "transaction_id"] - if not any(field in report for field in optional_fields): + optional_fields = {"ip_address", "maxmind_id", "minfraud_id", "transaction_id"} + if not optional_fields & report.keys(): # We return MultipleInvalid instead of ValueError to be consistent with what # voluptuous returns. raise MultipleInvalid( From b4a69b0310c20c5f494d5a3d2d51beb8d431e1b8 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Thu, 6 Jun 2024 20:51:25 +0000 Subject: [PATCH 5/5] Disallow empty uuid as minfraud_id for transaction report --- minfraud/validation.py | 15 ++++++++++++--- tests/test_validation.py | 1 + 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index fe9c0d3..dd25f74 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -379,6 +379,15 @@ def _uuid(s: str) -> str: raise ValueError +NIL_UUID = str(uuid.UUID(int=0)) + + +def _non_empty_uuid(s: str) -> str: + if _uuid(s) == NIL_UUID: + raise ValueError + return s + + def _transaction_id(s: Optional[str]) -> str: if isinstance(s, str) and len(s) > 0: return s @@ -390,7 +399,7 @@ def _transaction_id(s: Optional[str]) -> str: "chargeback_code": str, "ip_address": _ip_address, "maxmind_id": _maxmind_id, - "minfraud_id": _uuid, + "minfraud_id": _non_empty_uuid, "notes": str, Required("tag"): _tag, "transaction_id": _transaction_id, @@ -399,8 +408,8 @@ def _transaction_id(s: Optional[str]) -> str: def _validate_at_least_one_identifier_field(report): - optional_fields = {"ip_address", "maxmind_id", "minfraud_id", "transaction_id"} - if not optional_fields & report.keys(): + optional_fields = ["ip_address", "maxmind_id", "minfraud_id", "transaction_id"] + if not any(field in report for field in optional_fields): # We return MultipleInvalid instead of ValueError to be consistent with what # voluptuous returns. raise MultipleInvalid( diff --git a/tests/test_validation.py b/tests/test_validation.py index a57d9d6..7c94cff 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -420,6 +420,7 @@ def test_minfraud_id(self): "12345678-123412341234-12345678901", "12345678-1234-1234-1234-1234567890123", "12345678-1234-1234-1234-12345678901g", + "00000000-0000-0000-0000-000000000000", "", ): self.check_invalid_report({"minfraud_id": bad})