Skip to content

STF-2006: Add email verification inputs #476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
horgh merged 4 commits into from
Oct 27, 2026
Merged

STF-2006: Add email verification inputs #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1013a4a
Add email verification inputs
oschwald Oct 9, 2026
463ab2c
Test that email hashing keeps verification inputs
oschwald Oct 26, 2026
4145164
Use a recent event time in examples
oschwald Oct 26, 2026
0b20a07
Share the RFC 3339 time validation tests
oschwald Oct 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ History
``phone_was_verification_successful``, and ``phone_verification_time``
inputs to the ``/billing`` and ``/shipping`` request objects. They describe
the most recent verification of the phone number.
* Added the ``verification_time`` and ``was_verification_successful`` inputs
to the ``/email`` request object. They describe the most recent

@horgh horgh Oct 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this bullet does not say that verification_time must be an RFC 3339 string. Python users can easily pass a datetime object.

For example, {"email": {"verification_time": datetime.now(timezone.utc)}} fails validation with InvalidRequestError (NON_STRING_TIMES in the tests confirms this). With validate=False, it fails with a JSON serialization TypeError. This client has no per-field docstrings, so HISTORY.rst and the README are the only places that can state the format. The phone bullet above has the same gap for phone_verification_time.

Suggested fix: add a sentence such as "verification_time must be an RFC 3339 date-time string."

🤖 Comment by Claude Opus 5.5.

@horgh horgh Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's necessary

verification of the email address.
* The RFC 3339 check on ``/event/time`` and ``phone_verification_time`` now
rejects more values that the web service rejects, such as February 30,
hour 24, an offset of 24 hours or more, and a lowercase ``t`` or ``z``.
Expand Down
6 changes: 4 additions & 2 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,17 @@ Score, Insights and Factors Example
>>> 'shop_id': 's2123',
>>> 'type': 'purchase',
>>> 'transaction_id': 'txn3134133',
>>> 'time': '2014-04-12T23:20:50.052+00:00'
>>> 'time': '2026-10-05T23:20:50.052+00:00'
>>> },
>>> 'account': {
>>> 'user_id': '3132',
>>> 'username_md5': '570a90bfbf8c7eab5dc5d4e26832d5b1'
>>> },
>>> 'email': {
>>> 'address': '977577b140bfb7c516e4746204fbdb01',
>>> 'domain': 'maxmind.com'
>>> 'domain': 'maxmind.com',
>>> 'verification_time': '2026-10-01T14:25:00Z',

@horgh horgh Oct 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: in this example, verification_time (2026-10-01) is 12 years after the example event.time (2014-04-12, line 164). The "most recent verification" then comes after the transaction that it describes. tests/data/full-transaction-request.json line 16 has the same mismatch.

The phone phone_verification_time values (README lines 189 and 205, fixture lines 32 and 48) have the same problem. They are not new in this PR.

Suggested fix: use verification times before the event time, or move event.time to a recent date.

🤖 Comment by Claude Opus 5.5.

@oschwald oschwald Oct 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reply by Claude Opus 5.5 on Greg's behalf.

Fixed in 4145164. The example event time is now 2026-10-05, after every verification time. This also fixes the phone examples.

>>> 'was_verification_successful': True,
>>> },
>>> 'billing': {
>>> 'first_name': 'Jane',
Expand Down
2 changes: 2 additions & 0 deletions src/minfraud/validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,8 @@ def _uri(s: str) -> str:
"email": {
"address": _email_or_md5,
"domain": _hostname,
"verification_time": _rfc3339_datetime,
"was_verification_successful": bool,
},
"event": {
"party": _event_party,
Expand Down
6 changes: 4 additions & 2 deletions tests/data/full-transaction-request.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"party": "customer",
"transaction_id": "txn3134133",
"shop_id": "s2123",
"time": "2014-04-12T23:20:50.052+00:00",
"time": "2026-10-05T23:20:50.052+00:00",
"type": "purchase"
},
"account": {
Expand All @@ -12,7 +12,9 @@
},
"email": {
"address": "977577b140bfb7c516e4746204fbdb01",
"domain": "maxmind.com"
"domain": "maxmind.com",
"verification_time": "2026-10-01T14:25:00Z",
"was_verification_successful": true
},
"billing": {
"first_name": "First",
Expand Down
18 changes: 18 additions & 0 deletions tests/test_request.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,24 @@ def test_maybe_hash_email(self) -> None:
},
},
},
{
"name": "verification inputs kept",
"input": {
"email": {
"address": "test@maxmind.com",
"verification_time": "2026-10-01T14:25:00Z",
"was_verification_successful": False,
},
},
"expected": {
"email": {
"address": "977577b140bfb7c516e4746204fbdb01",
"domain": "maxmind.com",
"verification_time": "2026-10-01T14:25:00Z",
"was_verification_successful": False,
},
},
},
{
"name": "uppercase and alias",
"input": {"email": {"address": "Test+ignored@MaxMind.com"}},
Expand Down
34 changes: 14 additions & 20 deletions tests/test_validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ def check_bool(self, obj: str, key: str) -> None:
for bad in ("", 0, "True"):
self.check_invalid_transaction({obj: {key: bad}})

def check_rfc3339_time(self, obj: str, key: str) -> None:
for good in GOOD_TIMES:
self.check_transaction({obj: {key: good}})
for bad in BAD_TIMES + NON_STRING_TIMES:
self.check_invalid_transaction({obj: {key: bad}})

def setup_report(self, report: dict[str, Any]) -> None:
if "ip_address" not in report:
report["ip_address"] = "1.2.3.4"
Expand Down Expand Up @@ -186,18 +192,7 @@ def test_phone_was_verification_successful(self) -> None:
self.check_bool(self.type, "phone_was_verification_successful")

def test_phone_verification_time(self) -> None:
for good in GOOD_TIMES:
self.check_transaction({self.type: {"phone_verification_time": good}})
for bad in BAD_TIMES:
self.check_invalid_transaction(
{self.type: {"phone_verification_time": bad}},
)

def test_phone_verification_time_not_string(self) -> None:
for bad in NON_STRING_TIMES:
self.check_invalid_transaction(
{self.type: {"phone_verification_time": bad}},
)
self.check_rfc3339_time(self.type, "phone_verification_time")


class TestBillingAddress(AddressBase):
Expand Down Expand Up @@ -371,6 +366,12 @@ def test_domain(self) -> None:
for bad in ("bad ", " bad.com"):
self.check_invalid_transaction({"email": {"domain": bad}})

def test_verification_time(self) -> None:

@horgh horgh Oct 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: these two tests are the third copy of the GOOD_TIMES / BAD_TIMES / NON_STRING_TIMES loops. test_time / test_time_not_string (event) and test_phone_verification_time / test_phone_verification_time_not_string have the same loops. That code is not new in this PR.

If someone later adds a new list of bad values to one copy, the other fields do not get that coverage. Each new RFC 3339 input also needs two more copied methods.

Suggested fix: add a check_rfc3339_time(obj, key) helper next to check_bool, and use it for all three fields, for example self.check_rfc3339_time("email", "verification_time").

🤖 Comment by Claude Opus 5.5.

@oschwald oschwald Oct 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reply by Claude Opus 5.5 on Greg's behalf.

Fixed in 0b20a07. A check_rfc3339_time(obj, key) helper next to check_bool now covers the event, phone, and email verification times.

self.check_rfc3339_time("email", "verification_time")

def test_was_verification_successful(self) -> None:
self.check_bool("email", "was_verification_successful")


class TestEvent(ValidationBase, unittest.TestCase):
def test_party(self) -> None:
Expand All @@ -386,14 +387,7 @@ def test_shop_id(self) -> None:
self.check_transaction_str_type("event", "shop_id")

def test_time(self) -> None:
for good in GOOD_TIMES:
self.check_transaction({"event": {"time": good}})
for bad in BAD_TIMES:
self.check_invalid_transaction({"event": {"time": bad}})

def test_time_not_string(self) -> None:
for bad in NON_STRING_TIMES:
self.check_invalid_transaction({"event": {"time": bad}})
self.check_rfc3339_time("event", "time")

def test_time_impossible_date_message(self) -> None:
transaction: dict[str, Any] = {"event": {"time": "2026-02-30T10:00:00Z"}}
Expand Down
10 changes: 9 additions & 1 deletion tests/test_webservice.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ def test_200_with_email_hashing(self) -> None:
"email": {
"address": "977577b140bfb7c516e4746204fbdb01",
"domain": "maxmind.com",
"verification_time": "2026-10-01T14:25:00Z",
"was_verification_successful": False,
},
},
).respond_with_data(
Expand All @@ -292,7 +294,13 @@ def test_200_with_email_hashing(self) -> None:
status=200,
)

request = {"email": {"address": "Test+ignore@maxmind.com"}}
request = {

@horgh horgh Oct 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this test now is the only check that maybe_hash_email keeps the other email keys. It checks this only through the pytest-httpserver JSON match. The table-driven test_maybe_hash_email in tests/test_request.py has no case with extra email keys.

If a later change to maybe_hash_email drops or changes verification_time or was_verification_successful, no handler matches. The server returns 500 and the test fails with a generic HTTPError, not a dict diff, so the cause is hard to find.

Suggested fix: add a row to test_maybe_hash_email with these keys in both the input and the expected output. Then this test can stay address-only, as it was before.

🤖 Comment by Claude Opus 5.5.

@oschwald oschwald Oct 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reply by Claude Opus 5.5 on Greg's behalf.

Fixed in 463ab2c. test_maybe_hash_email now has a row with both verification keys in the input and the expected output.

"email": {
"address": "Test+ignore@maxmind.com",
"verification_time": "2026-10-01T14:25:00Z",
"was_verification_successful": False,
},
}
self.run_client(getattr(self.client, self.type)(request, hash_email=True))

# This was fixed in https://github.com/maxmind/minfraud-api-python/pull/78
Expand Down
Loading