From 3049f446a40db655445513085ac1ee83da119100 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 09:22:13 -0700 Subject: [PATCH 01/23] Don't explicitly inherit from object --- minfraud/models.py | 28 ++++++++++++++-------------- minfraud/webservice.py | 2 +- tests/test_validation.py | 2 +- tests/test_webservice.py | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/minfraud/models.py b/minfraud/models.py index 0cd129f..803690d 100644 --- a/minfraud/models.py +++ b/minfraud/models.py @@ -208,7 +208,7 @@ def __setattr__(self, name, value): @_inflate_to_namedtuple -class ScoreIPAddress(object): +class ScoreIPAddress: """Information about the IP address for minFraud Score. .. attribute:: risk @@ -226,7 +226,7 @@ class ScoreIPAddress(object): @_inflate_to_namedtuple -class Issuer(object): +class Issuer: """Information about the credit card issuer. .. attribute:: name @@ -274,7 +274,7 @@ class Issuer(object): @_inflate_to_namedtuple -class Device(object): +class Device: """Information about the device associated with the IP address. In order to receive device output from minFraud Insights or minFraud @@ -327,7 +327,7 @@ class Device(object): @_inflate_to_namedtuple -class Disposition(object): +class Disposition: """Information about disposition for the request as set by custom rules. In order to receive a disposition, you must be use the minFraud custom @@ -358,7 +358,7 @@ class Disposition(object): @_inflate_to_namedtuple -class EmailDomain(object): +class EmailDomain: """Information about the email domain passed in the request. .. attribute:: first_seen @@ -378,7 +378,7 @@ class EmailDomain(object): @_inflate_to_namedtuple -class Email(object): +class Email: """Information about the email address passed in the request. .. attribute:: domain @@ -431,7 +431,7 @@ class Email(object): @_inflate_to_namedtuple -class CreditCard(object): +class CreditCard: """Information about the credit card based on the issuer ID number. .. attribute:: country @@ -508,7 +508,7 @@ class CreditCard(object): @_inflate_to_namedtuple -class BillingAddress(object): +class BillingAddress: """Information about the billing address. .. attribute:: distance_to_ip_location @@ -562,7 +562,7 @@ class BillingAddress(object): @_inflate_to_namedtuple -class ShippingAddress(object): +class ShippingAddress: """Information about the shipping address. .. attribute:: distance_to_ip_location @@ -635,7 +635,7 @@ class ShippingAddress(object): @_inflate_to_namedtuple -class ServiceWarning(object): +class ServiceWarning: """Warning from the web service. .. attribute:: code @@ -674,7 +674,7 @@ class ServiceWarning(object): @_inflate_to_namedtuple -class Subscores(object): +class Subscores: """Subscores used in calculating the overall risk score. .. attribute:: avs_result @@ -869,7 +869,7 @@ class Subscores(object): @_inflate_to_namedtuple -class Factors(object): +class Factors: """Model for Factors response. .. attribute:: id @@ -985,7 +985,7 @@ class Factors(object): @_inflate_to_namedtuple -class Insights(object): +class Insights: """Model for Insights response. .. attribute:: id @@ -1095,7 +1095,7 @@ class Insights(object): @_inflate_to_namedtuple -class Score(object): +class Score: """Model for Score response. .. attribute:: id diff --git a/minfraud/webservice.py b/minfraud/webservice.py index 18a18b7..c5ffd7d 100644 --- a/minfraud/webservice.py +++ b/minfraud/webservice.py @@ -23,7 +23,7 @@ from .validation import validate_report, validate_transaction -class Client(object): +class Client: """Client for accessing the minFraud Score and Insights web services.""" def __init__( diff --git a/tests/test_validation.py b/tests/test_validation.py index 72e795c..d8590fb 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -14,7 +14,7 @@ unittest.TestCase.assertRegex = unittest.TestCase.assertRegexpMatches -class ValidationBase(object): +class ValidationBase: def setup_transaction(self, transaction): if "device" not in transaction: transaction["device"] = {} diff --git a/tests/test_webservice.py b/tests/test_webservice.py index 65046a3..90e34a4 100644 --- a/tests/test_webservice.py +++ b/tests/test_webservice.py @@ -25,7 +25,7 @@ unittest.TestCase.assertRegex = unittest.TestCase.assertRegexpMatches -class BaseTest(object): +class BaseTest: def setUp(self): self.client = Client(42, "abcdef123456") From 728c8844a2998b71fb3865494a5d9c0515bb82f8 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 09:23:00 -0700 Subject: [PATCH 02/23] Do not test on 2.7, 3.5, and PyPy --- .travis.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9a1c98c..dd6b2a7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,23 +3,14 @@ sudo: false language: python matrix: include: - - python: 2.7 - dist: trusty - - python: 3.5 - dist: trusty - python: 3.6 - dist: trusty - python: 3.7 - dist: xenial - python: 3.8 - dist: xenial env: - RUN_LINTER=1 - RUN_SNYK=1 - python: nightly dist: xenial - - python: pypy - dist: trusty allow_failures: - python: nightly From d0a8faec234d8bc9cedce97089c1d711272be064 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 09:25:06 -0700 Subject: [PATCH 03/23] Updated supported Python versions in docs and setup.py --- README.rst | 5 ++--- setup.py | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/README.rst b/README.rst index 45ae664..f0153c9 100644 --- a/README.rst +++ b/README.rst @@ -5,7 +5,7 @@ minFraud Score, Insights, Factors and Report Transaction Python API Description ----------- -This package provides an API for the `MaxMind minFraud Score, Insights, and +This package provides an API for the `MaxMind minFraud Score, Insights, and Factors web services `_ as well as the `Report Transaction web service `_. @@ -252,8 +252,7 @@ Report Transactions Example Requirements ------------ -This code requires Python 2.7+ or 3.5+. Older versions are not supported. -This library has been tested with CPython and PyPy. +Python 3.6 or greater is required. Older versions are not supported. Versioning ---------- diff --git a/setup.py b/setup.py index 7a6d998..71a86bb 100644 --- a/setup.py +++ b/setup.py @@ -54,9 +54,7 @@ "Environment :: Web Environment", "Intended Audience :: Developers", "License :: OSI Approved :: Apache Software License", - "Programming Language :: Python :: 2.7", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", From f5ba44ae2f06d2a834bdf9fd5e8862f2f496053d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 09:27:01 -0700 Subject: [PATCH 04/23] Require geoip2 4.0.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 71a86bb..69da2aa 100644 --- a/setup.py +++ b/setup.py @@ -17,7 +17,7 @@ _readme = f.read() requirements = [ - "geoip2>=3.0.0,<4.0.0", + "geoip2>=4.0.0,<5.0.0", "requests>=2.22.0", "rfc3987", "strict-rfc3339", From 53dc944fe57df8b1c4eb266e7f674d0a2fc9e6af Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 09:29:59 -0700 Subject: [PATCH 05/23] Do not depend on geoip2.compat --- minfraud/validation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index ba0281f..6476fd0 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -1,12 +1,12 @@ """This is an internal module used for validating the minFraud request.""" +import ipaddress import re import sys import uuid from decimal import Decimal import rfc3987 -from geoip2.compat import compat_ip_address from strict_rfc3339 import validate_rfc3339 # I can't reproduce the failure locally and the order looks right to me. @@ -65,7 +65,7 @@ def _ip_address(s): # ipaddress accepts numeric IPs, which we don't want. if isinstance(s, (str, _unicode)) and not re.match(r"^\d+$", s): - return str(compat_ip_address(s)) + return str(ipaddress.ip_address(s)) raise ValueError From 26f8efaeeb1e1c393723fec4acebb0fc078ed813 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 09:35:02 -0700 Subject: [PATCH 06/23] Remove Python 2 compat validation code --- minfraud/validation.py | 80 ++++++++++++++++++------------------------ setup.cfg | 4 +++ 2 files changed, 39 insertions(+), 45 deletions(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index 6476fd0..97cc908 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -2,7 +2,6 @@ import ipaddress import re -import sys import uuid from decimal import Decimal @@ -30,41 +29,32 @@ # # pylint: disable=invalid-name,undefined-variable -if sys.version_info[0] >= 3: - _unicode = str - _unicode_or_printable_ascii = str - long = int -else: - _unicode = unicode - _unicode_or_printable_ascii = Any(unicode, Match(r"^[\x20-\x7E]*$")) +_any_number = Any(float, int, Decimal) -_any_string = Any(_unicode_or_printable_ascii, str) -_any_number = Any(float, int, long, Decimal) - -_custom_input_key = All(_any_string, Match(r"^[a-z0-9_]{1,25}$")) +_custom_input_key = All(str, Match(r"^[a-z0-9_]{1,25}$")) _custom_input_value = Any( - All(_any_string, Match(r"^[^\n]{1,255}\Z")), + All(str, Match(r"^[^\n]{1,255}\Z")), All( _any_number, Range(min=-1e13, max=1e13, min_included=False, max_included=False) ), bool, ) -_md5 = All(_any_string, Match(r"^[0-9A-Fa-f]{32}$")) +_md5 = All(str, Match(r"^[0-9A-Fa-f]{32}$")) -_country_code = All(_any_string, Match(r"^[A-Z]{2}$")) +_country_code = All(str, Match(r"^[A-Z]{2}$")) _telephone_country_code = Any( - All(_any_string, Match("^[0-9]{1,4}$")), All(int, Range(min=1, max=9999)) + All(str, Match("^[0-9]{1,4}$")), All(int, Range(min=1, max=9999)) ) -_subdivision_iso_code = All(_any_string, Match(r"^[0-9A-Z]{1,4}$")) +_subdivision_iso_code = All(str, Match(r"^[0-9A-Z]{1,4}$")) def _ip_address(s): # ipaddress accepts numeric IPs, which we don't want. - if isinstance(s, (str, _unicode)) and not re.match(r"^\d+$", s): + if isinstance(s, str) and not re.match(r"^\d+$", s): return str(ipaddress.ip_address(s)) raise ValueError @@ -89,16 +79,16 @@ def _hostname(hostname): _delivery_speed = In(["same_day", "overnight", "expedited", "standard"]) _address = { - "address": _unicode_or_printable_ascii, - "address_2": _unicode_or_printable_ascii, - "city": _unicode_or_printable_ascii, - "company": _unicode_or_printable_ascii, + "address": str, + "address_2": str, + "city": str, + "company": str, "country": _country_code, - "first_name": _unicode_or_printable_ascii, - "last_name": _unicode_or_printable_ascii, + "first_name": str, + "last_name": str, "phone_country_code": _telephone_country_code, - "phone_number": _unicode_or_printable_ascii, - "postal": _unicode_or_printable_ascii, + "phone_number": str, + "postal": str, "region": _subdivision_iso_code, } @@ -287,18 +277,18 @@ def _uri(s): validate_transaction = Schema( { - "account": {"user_id": _unicode_or_printable_ascii, "username_md5": _md5,}, + "account": {"user_id": str, "username_md5": _md5,}, "billing": _address, "payment": { "processor": _payment_processor, "was_authorized": bool, - "decline_code": _unicode_or_printable_ascii, + "decline_code": str, }, "credit_card": { "avs_result": _single_char, - "bank_name": _unicode_or_printable_ascii, + "bank_name": str, "bank_phone_country_code": _telephone_country_code, - "bank_phone_number": _unicode_or_printable_ascii, + "bank_phone_number": str, "cvv_result": _single_char, "issuer_id_number": _iin, "last_4_digits": _credit_card_last_4, @@ -306,34 +296,34 @@ def _uri(s): }, "custom_inputs": {_custom_input_key: _custom_input_value}, Required("device"): { - "accept_language": _unicode_or_printable_ascii, + "accept_language": str, Required("ip_address"): _ip_address, "session_age": All(_any_number, Range(min=0)), - "session_id": _unicode_or_printable_ascii, - "user_agent": _unicode_or_printable_ascii, + "session_id": str, + "user_agent": str, }, "email": {"address": _email_or_md5, "domain": _hostname,}, "event": { - "shop_id": _unicode_or_printable_ascii, + "shop_id": str, "time": _rfc3339_datetime, "type": _event_type, - "transaction_id": _unicode_or_printable_ascii, + "transaction_id": str, }, "order": { - "affiliate_id": _unicode_or_printable_ascii, + "affiliate_id": str, "amount": _price, "currency": _currency_code, - "discount_code": _unicode_or_printable_ascii, + "discount_code": str, "has_gift_message": bool, "is_gift": bool, "referrer_uri": _uri, - "subaffiliate_id": _unicode_or_printable_ascii, + "subaffiliate_id": str, }, "shipping": _shipping_address, "shopping_cart": [ { - "category": _unicode_or_printable_ascii, - "item_id": _unicode_or_printable_ascii, + "category": str, + "item_id": str, "price": _price, "quantity": All(int, Range(min=1)), }, @@ -343,7 +333,7 @@ def _uri(s): def _maxmind_id(s): - if isinstance(s, (str, _unicode)) and len(s) == 8: + if isinstance(s, str) and len(s) == 8: return s raise ValueError @@ -354,19 +344,19 @@ def _maxmind_id(s): def _uuid(s): if isinstance(s, uuid.UUID): return str(s) - if isinstance(s, (str, _unicode)): + if isinstance(s, str): return str(uuid.UUID(s)) raise ValueError validate_report = Schema( { - "chargeback_code": _unicode_or_printable_ascii, + "chargeback_code": str, Required("ip_address"): _ip_address, "maxmind_id": _maxmind_id, "minfraud_id": _uuid, - "notes": _unicode_or_printable_ascii, + "notes": str, Required("tag"): _tag, - "transaction_id": _unicode_or_printable_ascii, + "transaction_id": str, }, ) diff --git a/setup.cfg b/setup.cfg index f038699..cdfe10f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -2,5 +2,9 @@ build_html = build_sphinx -b html --build-dir docs sdist = build_html sdist +[flake8] +# black uses 88 : ¯\_(ツ)_/¯ +max-line-length = 88 + [wheel] universal = 1 From 17a64791b08ef8b131edb92a8e8146709e86344a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 09:36:50 -0700 Subject: [PATCH 07/23] Remove Python 2 unittest compat code --- tests/test_models.py | 9 +-------- tests/test_validation.py | 10 +--------- tests/test_webservice.py | 10 +--------- 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/tests/test_models.py b/tests/test_models.py index 7329292..52bc75c 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -2,14 +2,7 @@ from minfraud.models import * -if sys.version_info[:2] == (2, 6): - import unittest2 as unittest -else: - import unittest - -if sys.version_info[0] == 2: - unittest.TestCase.assertRaisesRegex = unittest.TestCase.assertRaisesRegexp - unittest.TestCase.assertRegex = unittest.TestCase.assertRegexpMatches +import unittest class TestModels(unittest.TestCase): diff --git a/tests/test_validation.py b/tests/test_validation.py index d8590fb..5c101d9 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -1,17 +1,9 @@ from decimal import Decimal -import sys from voluptuous import MultipleInvalid from minfraud.validation import validate_transaction, validate_report -if sys.version_info[:2] == (2, 6): - import unittest2 as unittest -else: - import unittest - -if sys.version_info[0] == 2: - unittest.TestCase.assertRaisesRegex = unittest.TestCase.assertRaisesRegexp - unittest.TestCase.assertRegex = unittest.TestCase.assertRegexpMatches +import unittest class ValidationBase: diff --git a/tests/test_webservice.py b/tests/test_webservice.py index 90e34a4..8bec76f 100644 --- a/tests/test_webservice.py +++ b/tests/test_webservice.py @@ -1,5 +1,4 @@ import os -import sys import json import requests_mock @@ -15,14 +14,7 @@ from minfraud.models import Factors, Insights, Score from minfraud.webservice import Client -if sys.version_info[:2] == (2, 6): - import unittest2 as unittest -else: - import unittest - -if sys.version_info[0] == 2: - unittest.TestCase.assertRaisesRegex = unittest.TestCase.assertRaisesRegexp - unittest.TestCase.assertRegex = unittest.TestCase.assertRegexpMatches +import unittest class BaseTest: From 310bf98f55f4056ee47d98b23a367bdda90f1459 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 09:44:05 -0700 Subject: [PATCH 08/23] Fix several minor pylint issues --- minfraud/validation.py | 18 +++++++++--------- minfraud/webservice.py | 12 ++++++++---- pylintrc | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index 97cc908..c4b7471 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -1,4 +1,12 @@ -"""This is an internal module used for validating the minFraud request.""" +"""This is an internal module used for validating the minFraud request. + +Internal code for validating the transaction dictionary. + +This code is only intended for internal use and is subject to change in ways +that may break any direct use of it. + +""" + import ipaddress import re @@ -15,14 +23,6 @@ from validate_email import validate_email from voluptuous import All, Any, In, Match, Range, Required, Schema -""" -Internal code for validating the transaction dictionary. - -This code is only intended for internal use and is subject to change in ways -that may break any direct use of it. - -""" - # Pylint doesn't like the private function type naming for the callable # objects below. Given the consistent use of them, the current names seem # preferable to blindly following pylint. diff --git a/minfraud/webservice.py b/minfraud/webservice.py index c5ffd7d..554adc4 100644 --- a/minfraud/webservice.py +++ b/minfraud/webservice.py @@ -195,7 +195,8 @@ def _copy_and_clean(self, data): return [self._copy_and_clean(x) for x in data if x is not None] return data - def _user_agent(self): + @staticmethod + def _user_agent(): """Create User-Agent header.""" return "minFraud-API/%s %s" % (__version__, default_user_agent()) @@ -261,7 +262,8 @@ def _exception_for_4xx_status(self, response, status, uri): uri, ) - def _exception_for_web_service_error(self, message, code, status, uri): + @staticmethod + def _exception_for_web_service_error(message, code, status, uri): """Returns exception for error responses with the JSON body.""" if code in ( "ACCOUNT_ID_REQUIRED", @@ -277,7 +279,8 @@ def _exception_for_web_service_error(self, message, code, status, uri): return InvalidRequestError(message, code, status, uri) - def _exception_for_5xx_status(self, status, uri): + @staticmethod + def _exception_for_5xx_status(status, uri): """Returns exception for error response with 5xx status codes.""" return HTTPError( u"Received a server error ({0}) for " u"{1}".format(status, uri), @@ -285,7 +288,8 @@ def _exception_for_5xx_status(self, status, uri): uri, ) - def _exception_for_unexpected_status(self, status, uri): + @staticmethod + def _exception_for_unexpected_status(status, uri): """Returns exception for responses with unexpected status codes.""" return HTTPError( u"Received an unexpected HTTP status " u"({0}) for {1}".format(status, uri), diff --git a/pylintrc b/pylintrc index 4c925b6..32fff1f 100644 --- a/pylintrc +++ b/pylintrc @@ -1,5 +1,5 @@ [MESSAGES CONTROL] -disable=C0330,R0201,R0205,W0105,locally-disabled,too-few-public-methods +disable=bad-continuation,too-few-public-methods [BASIC] From 4452f11b47753cd0ca1afc1bf284f8d5c5dd9f42 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 10:08:35 -0700 Subject: [PATCH 09/23] Add types for model classes (and fix some doc types) --- minfraud/models.py | 135 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 125 insertions(+), 10 deletions(-) diff --git a/minfraud/models.py b/minfraud/models.py index 803690d..585323f 100644 --- a/minfraud/models.py +++ b/minfraud/models.py @@ -8,6 +8,7 @@ # pylint:disable=too-many-lines from collections import namedtuple from functools import update_wrapper +from typing import List, Optional import geoip2.models import geoip2.records @@ -84,7 +85,9 @@ class GeoIP2Location(geoip2.records.Location): """ - __doc__ += geoip2.records.Location.__doc__ + __doc__ += geoip2.records.Location.__doc__ # type: ignore + + local_time: Optional[str] def __init__(self, *args, **kwargs): self.local_time = kwargs.get("local_time", None) @@ -110,7 +113,9 @@ class GeoIP2Country(geoip2.records.Country): """ - __doc__ += geoip2.records.Country.__doc__ + __doc__ += geoip2.records.Country.__doc__ # type: ignore + + is_high_risk: bool def __init__(self, *args, **kwargs): self.is_high_risk = kwargs.get("is_high_risk", False) @@ -187,6 +192,10 @@ class IPAddress(geoip2.models.Insights): """ + country: GeoIP2Country + location: GeoIP2Location + risk: Optional[float] + def __init__(self, ip_address): if ip_address is None: ip_address = {} @@ -219,6 +228,8 @@ class ScoreIPAddress: :type: float | None """ + risk: Optional[float] + __slots__ = () _fields = { "risk": None, @@ -243,14 +254,14 @@ class Issuer: no issuer ID number (IIN) was provided in the request or if MaxMind does not have a name associated with the IIN. - :type: bool + :type: bool | None .. attribute:: phone_number The phone number of the bank which issued the credit card. In some cases the phone number we return may be out of date. - :type: str + :type: str | None .. attribute:: matches_provided_phone_number @@ -260,10 +271,15 @@ class Issuer: phone number or no issuer ID number (IIN) was provided in the request or if MaxMind does not have a phone number associated with the IIN. - :type: bool + :type: bool | None """ + name: Optional[str] + matches_provided_name: Optional[bool] + phone_number: Optional[str] + matches_provided_phone_number: Optional[bool] + __slots__ = () _fields = { "name": None, @@ -317,6 +333,11 @@ class Device: """ + confidence: Optional[float] + id: Optional[str] + last_seen: Optional[str] + local_time: Optional[str] + __slots__ = () _fields = { "confidence": None, @@ -350,6 +371,9 @@ class Disposition: :type: str | None """ + action: Optional[str] + reason: Optional[str] + __slots__ = () _fields = { "action": None, @@ -367,10 +391,12 @@ class EmailDomain: was first seen by MaxMind. This is expressed using the ISO 8601 date format. - :type: str + :type: str | None """ + first_seen: Optional[str] + __slots__ = () _fields = { "first_seen": None, @@ -393,7 +419,7 @@ class Email: was first seen by MaxMind. This is expressed using the ISO 8601 date format. - :type: str + :type: str | None .. attribute:: is_disposable @@ -420,6 +446,12 @@ class Email: """ + domain: EmailDomain + first_seen: Optional[str] + is_disposable: Optional[bool] + is_free: Optional[bool] + is_high_risk: Optional[bool] + __slots__ = () _fields = { "domain": EmailDomain, @@ -494,6 +526,15 @@ class CreditCard: """ + issuer: Issuer + country: Optional[str] + brand: Optional[str] + is_business: Optional[bool] + is_issued_in_billing_address_country: Optional[bool] + is_prepaid: Optional[bool] + is_virtual: Optional[bool] + type: Optional[str] + __slots__ = () _fields = { "issuer": Issuer, @@ -551,6 +592,12 @@ class BillingAddress: """ + is_postal_in_city: Optional[bool] + latitude: Optional[float] + longitude: Optional[float] + distance_to_ip_location: Optional[int] + is_in_ip_country: Optional[bool] + __slots__ = () _fields = { "is_postal_in_city": None, @@ -622,6 +669,14 @@ class ShippingAddress: """ + is_postal_in_city: Optional[bool] + latitude: Optional[float] + longitude: Optional[float] + distance_to_ip_location: Optional[int] + is_in_ip_country: Optional[bool] + is_high_risk: Optional[bool] + distance_to_billing_address: Optional[int] + __slots__ = () _fields = { "is_postal_in_city": None, @@ -645,7 +700,7 @@ class ServiceWarning: `_ for the current list of of warning codes. - :type: str + :type: str | None .. attribute:: warning @@ -653,7 +708,7 @@ class ServiceWarning: warning. The description may change at any time and should not be matched against. - :type: str + :type: str | None .. attribute:: input_pointer @@ -661,10 +716,14 @@ class ServiceWarning: the warning is associated with. For instance, if the warning was about the billing city, the string would be ``"/billing/city"``. - :type: str + :type: str | None """ + code: Optional[str] + warning: Optional[str] + input_pointer: Optional[str] + __slots__ = () _fields = { "code": None, @@ -843,6 +902,27 @@ class Subscores: """ + avs_result: Optional[float] + billing_address: Optional[float] + billing_address_distance_to_ip_location: Optional[float] + browser: Optional[float] + chargeback: Optional[float] + country: Optional[float] + country_mismatch: Optional[float] + cvv_result: Optional[float] + device: Optional[float] + email_address: Optional[float] + email_domain: Optional[float] + email_local_part: Optional[float] + email_tenure: Optional[float] + ip_tenure: Optional[float] + issuer_id_number: Optional[float] + order_amount: Optional[float] + phone_number: Optional[float] + shipping_address: Optional[float] + shipping_address_distance_to_ip_location: Optional[float] + time_of_day: Optional[float] + __slots__ = () _fields = { "avs_result": None, @@ -966,6 +1046,20 @@ class Factors: individual components that are used to calculate the overall risk score. """ + billing_address: BillingAddress + credit_card: CreditCard + disposition: Disposition + funds_remaining: Optional[float] + device: Device + email: Email + id: Optional[str] + ip_address: IPAddress + queries_remaining: Optional[int] + risk_score: Optional[float] + shipping_address: ShippingAddress + subscores: Subscores + warnings: List[ServiceWarning] + __slots__ = () _fields = { "billing_address": BillingAddress, @@ -1077,6 +1171,19 @@ class Insights: minFraud data related to the shipping address used in the transaction. """ + billing_address: BillingAddress + credit_card: CreditCard + device: Device + disposition: Disposition + email: Email + funds_remaining: Optional[float] + id: Optional[str] + ip_address: IPAddress + queries_remaining: Optional[int] + risk_score: Optional[float] + shipping_address: ShippingAddress + warnings: List[ServiceWarning] + __slots__ = () _fields = { "billing_address": BillingAddress, @@ -1153,6 +1260,14 @@ class Score: :type: IPAddress """ + disposition: Disposition + funds_remaining: Optional[float] + id: Optional[str] + ip_address: ScoreIPAddress + queries_remaining: Optional[int] + risk_score: Optional[float] + warnings: List[ServiceWarning] + __slots__ = () _fields = { "disposition": Disposition, From 6b78a6fc1b9df54467ae88b30fea3564941ca212 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 10:43:55 -0700 Subject: [PATCH 10/23] Get rid of user_id support Clean up Client constructor --- minfraud/webservice.py | 30 ++++++++---------------------- tests/test_webservice.py | 2 +- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/minfraud/webservice.py b/minfraud/webservice.py index 554adc4..d2aed42 100644 --- a/minfraud/webservice.py +++ b/minfraud/webservice.py @@ -26,17 +26,14 @@ class Client: """Client for accessing the minFraud Score and Insights web services.""" - def __init__( + def __init__( # pylint: disable=too-many-arguments self, - account_id=None, - license_key=None, - host="minfraud.maxmind.com", - locales=("en",), - timeout=None, - # This is deprecated and not documented for that reason. - # It can be removed if we do a major release in the future. - user_id=None, - ): + account_id: int, + license_key: str, + host: str = "minfraud.maxmind.com", + locales: Tuple[str] = ("en",), + timeout: None = None, + ) -> None: """Constructor for Client. :param account_id: Your MaxMind account ID @@ -52,22 +49,11 @@ def __init__( :return: Client object :rtype: Client """ - # pylint: disable=too-many-arguments - if account_id is None: - account_id = user_id - if account_id is None: - raise TypeError("The account_id is a required parameter") - if license_key is None: - raise TypeError("The license_key is a required parameter") - - # pylint: disable=too-many-arguments self._locales = locales # requests 2.12.2 requires that the username passed to auth be a # string - self._account_id = ( - account_id if isinstance(account_id, bytes) else str(account_id) - ) + self._account_id = str(account_id) self._license_key = license_key self._base_uri = u"https://{0:s}/minfraud/v2.0".format(host) self._timeout = timeout diff --git a/tests/test_webservice.py b/tests/test_webservice.py index 8bec76f..036e511 100644 --- a/tests/test_webservice.py +++ b/tests/test_webservice.py @@ -144,7 +144,7 @@ def test_named_constructor_args(self): key = "1234567890ab" for client in ( Client(account_id=id, license_key=key), - Client(user_id=id, license_key=key), + Client(account_id=id, license_key=key), ): self.assertEqual(client._account_id, id) self.assertEqual(client._license_key, key) From ad5d3ea54597b51ed41ddc90deefbae72e94fc8d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 10:58:40 -0700 Subject: [PATCH 11/23] Add types for other code --- minfraud/errors.py | 6 ++- minfraud/models.py | 22 +++++------ minfraud/validation.py | 17 +++++---- minfraud/webservice.py | 85 +++++++++++++++++++++++++++++++----------- tests/test_models.py | 2 +- 5 files changed, 89 insertions(+), 43 deletions(-) diff --git a/minfraud/errors.py b/minfraud/errors.py index 799cb46..43ce283 100644 --- a/minfraud/errors.py +++ b/minfraud/errors.py @@ -6,6 +6,8 @@ """ +from typing import Optional + class MinFraudError(RuntimeError): """There was a non-specific error in minFraud. @@ -39,7 +41,9 @@ class HTTPError(MinFraudError): """ - def __init__(self, message, http_status=None, uri=None): + def __init__( + self, message: str, http_status: Optional[int] = None, uri: Optional[str] = None + ) -> None: super(HTTPError, self).__init__(message) self.http_status = http_status self.uri = uri diff --git a/minfraud/models.py b/minfraud/models.py index 585323f..39ca22a 100644 --- a/minfraud/models.py +++ b/minfraud/models.py @@ -8,7 +8,7 @@ # pylint:disable=too-many-lines from collections import namedtuple from functools import update_wrapper -from typing import List, Optional +from typing import Any, Dict, List, Optional, Tuple import geoip2.models import geoip2.records @@ -59,12 +59,6 @@ def new(cls, *args, **kwargs): return new_cls -def _create_warnings(warnings): - if not warnings: - return () - return tuple([ServiceWarning(x) for x in warnings]) - - class GeoIP2Location(geoip2.records.Location): """Location information for the IP address. @@ -89,7 +83,7 @@ class GeoIP2Location(geoip2.records.Location): local_time: Optional[str] - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: self.local_time = kwargs.get("local_time", None) super(GeoIP2Location, self).__init__(*args, **kwargs) @@ -117,7 +111,7 @@ class GeoIP2Country(geoip2.records.Country): is_high_risk: bool - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: self.is_high_risk = kwargs.get("is_high_risk", False) super(GeoIP2Country, self).__init__(*args, **kwargs) @@ -196,7 +190,7 @@ class IPAddress(geoip2.models.Insights): location: GeoIP2Location risk: Optional[float] - def __init__(self, ip_address): + def __init__(self, ip_address: Dict[str, Any]) -> None: if ip_address is None: ip_address = {} locales = ip_address.get("_locales") @@ -210,7 +204,7 @@ def __init__(self, ip_address): # Unfortunately the GeoIP2 models are not immutable, only the records. This # corrects that for minFraud - def __setattr__(self, name, value): + def __setattr__(self, name: str, value: Any) -> None: if hasattr(self, "_finalized") and self._finalized: raise AttributeError("can't set attribute") super(IPAddress, self).__setattr__(name, value) @@ -732,6 +726,12 @@ class ServiceWarning: } +def _create_warnings(warnings: List[Dict[str, str]]) -> Tuple[ServiceWarning, ...]: + if not warnings: + return () + return tuple([ServiceWarning(x) for x in warnings]) + + @_inflate_to_namedtuple class Subscores: """Subscores used in calculating the overall risk score. diff --git a/minfraud/validation.py b/minfraud/validation.py index c4b7471..ee1f493 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -22,6 +22,7 @@ # pylint: disable=wrong-import-order from validate_email import validate_email from voluptuous import All, Any, In, Match, Range, Required, Schema +from typing import Optional # Pylint doesn't like the private function type naming for the callable # objects below. Given the consistent use of them, the current names seem @@ -52,14 +53,14 @@ _subdivision_iso_code = All(str, Match(r"^[0-9A-Z]{1,4}$")) -def _ip_address(s): +def _ip_address(s: Optional[str]) -> str: # ipaddress accepts numeric IPs, which we don't want. if isinstance(s, str) and not re.match(r"^\d+$", s): return str(ipaddress.ip_address(s)) raise ValueError -def _email_or_md5(s): +def _email_or_md5(s: str) -> str: if validate_email(s) or re.match(r"^[0-9A-Fa-f]{32}$", s): return s raise ValueError @@ -67,7 +68,7 @@ def _email_or_md5(s): # based off of: # http://stackoverflow.com/questions/2532053/validate-a-hostname-string -def _hostname(hostname): +def _hostname(hostname: str) -> str: if len(hostname) > 255: raise ValueError allowed = re.compile(r"(?!-)[A-Z\d-]{1,63}(? str: if re.match("^[\x21-\x7E]{1,255}$", s) and not re.match("^[0-9]{1,19}$", s): return s raise ValueError -def _rfc3339_datetime(s): +def _rfc3339_datetime(s: str) -> str: if validate_rfc3339(s): return s raise ValueError @@ -269,7 +270,7 @@ def _rfc3339_datetime(s): _price = All(_any_number, Range(min=0, min_included=False)) -def _uri(s): +def _uri(s: str) -> str: if rfc3987.parse(s).get("scheme") in ["http", "https"]: return s raise ValueError @@ -332,7 +333,7 @@ def _uri(s): ) -def _maxmind_id(s): +def _maxmind_id(s: Optional[str]) -> str: if isinstance(s, str) and len(s) == 8: return s raise ValueError @@ -341,7 +342,7 @@ def _maxmind_id(s): _tag = In(["chargeback", "not_fraud", "spam_or_abuse", "suspected_fraud"]) -def _uuid(s): +def _uuid(s: str) -> str: if isinstance(s, uuid.UUID): return str(s) if isinstance(s, str): diff --git a/minfraud/webservice.py b/minfraud/webservice.py index d2aed42..1979d49 100644 --- a/minfraud/webservice.py +++ b/minfraud/webservice.py @@ -6,7 +6,10 @@ """ +from typing import Any, cast, Dict, Optional, Tuple, Type, Union + import requests +from requests.models import Response from requests.utils import default_user_agent from voluptuous import MultipleInvalid @@ -32,7 +35,7 @@ def __init__( # pylint: disable=too-many-arguments license_key: str, host: str = "minfraud.maxmind.com", locales: Tuple[str] = ("en",), - timeout: None = None, + timeout: Optional[float] = None, ) -> None: """Constructor for Client. @@ -58,7 +61,7 @@ def __init__( # pylint: disable=too-many-arguments self._base_uri = u"https://{0:s}/minfraud/v2.0".format(host) self._timeout = timeout - def factors(self, transaction, validate=True): + def factors(self, transaction: Dict[str, Any], validate: bool = True) -> Factors: """Query Factors endpoint with transaction data. :param transaction: A dictionary containing the transaction to be @@ -76,9 +79,11 @@ def factors(self, transaction, validate=True): :raises: AuthenticationError, InsufficientFundsError, InvalidRequestError, HTTPError, MinFraudError, """ - return self._response_for("factors", Factors, transaction, validate) + return cast( + Factors, self._response_for("factors", Factors, transaction, validate) + ) - def insights(self, transaction, validate=True): + def insights(self, transaction: Dict[str, Any], validate: bool = True) -> Insights: """Query Insights endpoint with transaction data. :param transaction: A dictionary containing the transaction to be @@ -96,9 +101,11 @@ def insights(self, transaction, validate=True): :raises: AuthenticationError, InsufficientFundsError, InvalidRequestError, HTTPError, MinFraudError, """ - return self._response_for("insights", Insights, transaction, validate) + return cast( + Insights, self._response_for("insights", Insights, transaction, validate) + ) - def score(self, transaction, validate=True): + def score(self, transaction: Dict[str, Any], validate: bool = True) -> Score: """Query Score endpoint with transaction data. :param transaction: A dictionary containing the transaction to be @@ -116,9 +123,15 @@ def score(self, transaction, validate=True): :raises: AuthenticationError, InsufficientFundsError, InvalidRequestError, HTTPError, MinFraudError, """ - return self._response_for("score", Score, transaction, validate) + return cast(Score, self._response_for("score", Score, transaction, validate)) - def _response_for(self, path, model_class, request, validate): + def _response_for( + self, + path: str, + model_class: Union[Type[Factors], Type[Score], Type[Insights]], + request: Dict[str, Any], + validate: bool, + ) -> Union[Score, Factors, Insights]: """Send request and create response object.""" cleaned_request = self._copy_and_clean(request) if validate: @@ -132,7 +145,7 @@ def _response_for(self, path, model_class, request, validate): raise self._exception_for_error(response, uri) return self._handle_success(response, uri, model_class) - def report(self, report, validate=True): + def report(self, report: Dict[str, Optional[str]], validate: bool = True) -> None: """Send a transaction report to the Report Transaction endpoint. :param report: A dictionary containing the transaction report to be sent @@ -162,7 +175,7 @@ def report(self, report, validate=True): if response.status_code != 204: raise self._exception_for_error(response, uri) - def _do_request(self, uri, data): + def _do_request(self, uri: str, data: Dict[str, Any]) -> Response: return requests.post( uri, json=data, @@ -171,7 +184,7 @@ def _do_request(self, uri, data): timeout=self._timeout, ) - def _copy_and_clean(self, data): + def _copy_and_clean(self, data: Any) -> Any: """Create a copy of the data structure with Nones removed.""" if isinstance(data, dict): return dict( @@ -182,11 +195,16 @@ def _copy_and_clean(self, data): return data @staticmethod - def _user_agent(): + def _user_agent() -> str: """Create User-Agent header.""" return "minFraud-API/%s %s" % (__version__, default_user_agent()) - def _handle_success(self, response, uri, model_class): + def _handle_success( + self, + response: Response, + uri: str, + model_class: Union[Type[Factors], Type[Score], Type[Insights]], + ) -> Union[Score, Factors, Insights]: """Handle successful response.""" try: body = response.json() @@ -194,7 +212,7 @@ def _handle_success(self, response, uri, model_class): raise MinFraudError( "Received a 200 response" " but could not decode the response as " - "JSON: {0}".format(response.content), + "JSON: {0}".format(str(response.content)), 200, uri, ) @@ -202,7 +220,15 @@ def _handle_success(self, response, uri, model_class): body["ip_address"]["_locales"] = self._locales return model_class(body) - def _exception_for_error(self, response, uri): + def _exception_for_error( + self, response: Response, uri: str + ) -> Union[ + AuthenticationError, + InsufficientFundsError, + InvalidRequestError, + HTTPError, + PermissionRequiredError, + ]: """Returns the exception for the error responses.""" status = response.status_code @@ -212,7 +238,15 @@ def _exception_for_error(self, response, uri): return self._exception_for_5xx_status(status, uri) return self._exception_for_unexpected_status(status, uri) - def _exception_for_4xx_status(self, response, status, uri): + def _exception_for_4xx_status( + self, response: Response, status: int, uri: str + ) -> Union[ + AuthenticationError, + InsufficientFundsError, + InvalidRequestError, + HTTPError, + PermissionRequiredError, + ]: """Returns exception for error responses with 4xx status codes.""" if not response.content: return HTTPError( @@ -221,7 +255,7 @@ def _exception_for_4xx_status(self, response, status, uri): if response.headers.get("Content-Type", "").find("json") == -1: return HTTPError( "Received a {0} with the following " - "body: {1}".format(status, response.content), + "body: {1}".format(status, str(response.content)), status, uri, ) @@ -231,7 +265,7 @@ def _exception_for_4xx_status(self, response, status, uri): return HTTPError( "Received a {status:d} error but it did not include" " the expected JSON body: {content}".format( - status=status, content=response.content + status=status, content=str(response.content) ), status, uri, @@ -243,13 +277,20 @@ def _exception_for_4xx_status(self, response, status, uri): ) return HTTPError( "Error response contains JSON but it does not specify code" - " or error keys: {0}".format(response.content), + " or error keys: {0}".format(str(response.content)), status, uri, ) @staticmethod - def _exception_for_web_service_error(message, code, status, uri): + def _exception_for_web_service_error( + message: str, code: str, status: int, uri: str + ) -> Union[ + InvalidRequestError, + AuthenticationError, + PermissionRequiredError, + InsufficientFundsError, + ]: """Returns exception for error responses with the JSON body.""" if code in ( "ACCOUNT_ID_REQUIRED", @@ -266,7 +307,7 @@ def _exception_for_web_service_error(message, code, status, uri): return InvalidRequestError(message, code, status, uri) @staticmethod - def _exception_for_5xx_status(status, uri): + def _exception_for_5xx_status(status: int, uri: str) -> HTTPError: """Returns exception for error response with 5xx status codes.""" return HTTPError( u"Received a server error ({0}) for " u"{1}".format(status, uri), @@ -275,7 +316,7 @@ def _exception_for_5xx_status(status, uri): ) @staticmethod - def _exception_for_unexpected_status(status, uri): + def _exception_for_unexpected_status(status: int, uri: str) -> HTTPError: """Returns exception for responses with unexpected status codes.""" return HTTPError( u"Received an unexpected HTTP status " u"({0}) for {1}".format(status, uri), diff --git a/tests/test_models.py b/tests/test_models.py index 52bc75c..c9bdb15 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -27,7 +27,7 @@ def test_model_immutability(self): with self.assertRaises( AttributeError, msg="{0!s} - {0}".format(model.obj, attr) ): - setattr(model.obj, attr, 5) + setattr(model.obj, attr, 5) # type: ignore def test_billing_address(self): address = BillingAddress(self.address_dict) From 294a1740a2536c45b03ac7d3e61966ae15a154ba Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 11:49:01 -0700 Subject: [PATCH 12/23] Run mypy on Travis --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index dd6b2a7..1b11859 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,9 +17,10 @@ matrix: before_install: - "if [[ $RUN_SNYK && $SNYK_TOKEN ]]; then sudo apt-get install -y nodejs; npm install -g snyk; fi" install: - - "if [[ $RUN_LINTER ]]; then pip install --upgrade pylint black; fi" + - "if [[ $RUN_LINTER ]]; then pip install --upgrade pylint black mypy voluptuous-stubs; fi" script: - python setup.py test + - "if [[ $RUN_LINTER ]]; then mypy minfraud tests; fi" - "if [[ $RUN_LINTER ]]; then ./.travis-pylint.sh; fi" - "if [[ $RUN_LINTER ]]; then ./.travis-black.sh; fi" - "if [[ $RUN_SNYK && $SNYK_TOKEN ]]; then snyk test --org=maxmind --file=requirements.txt; fi" From ecdf1a6943ae2954e44c2fee4b57b2d88505c92a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 11:57:05 -0700 Subject: [PATCH 13/23] Replace validate_email email_validator validate_email has not been updated since 2015. --- HISTORY.rst | 5 +++++ dev-bin/release.sh | 2 +- minfraud/validation.py | 6 +++--- setup.py | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 64a731b..82a9ce2 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -3,6 +3,11 @@ History ------- +2.0.0 + +* Email validation is now done with ``email_validator`` rather than + ``validate_email``. + 1.13.0 (2020-07-14) +++++++++++++++++++ diff --git a/dev-bin/release.sh b/dev-bin/release.sh index 53157cc..e09c5e2 100755 --- a/dev-bin/release.sh +++ b/dev-bin/release.sh @@ -35,7 +35,7 @@ if [ -n "$(git status --porcelain)" ]; then fi # Make sure release deps are installed with the current python -pip install -U sphinx wheel voluptuous strict_rfc3339 validate_email rfc3987 +pip install -U sphinx wheel voluptuous strict_rfc3339 email_validator rfc3987 perl -pi -e "s/(?<=__version__ = \").+?(?=\")/$version/g" minfraud/version.py diff --git a/minfraud/validation.py b/minfraud/validation.py index ee1f493..560081e 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -20,7 +20,7 @@ # It is failing on pylint 1.8.3 on Travis. We should try removing this # when a new version of pylint is released. # pylint: disable=wrong-import-order -from validate_email import validate_email +from email_validator import validate_email from voluptuous import All, Any, In, Match, Range, Required, Schema from typing import Optional @@ -61,9 +61,9 @@ def _ip_address(s: Optional[str]) -> str: def _email_or_md5(s: str) -> str: - if validate_email(s) or re.match(r"^[0-9A-Fa-f]{32}$", s): + if re.match(r"^[0-9A-Fa-f]{32}$", s): return s - raise ValueError + return validate_email(s, check_deliverability=False).email # based off of: diff --git a/setup.py b/setup.py index 69da2aa..e61de85 100644 --- a/setup.py +++ b/setup.py @@ -17,12 +17,12 @@ _readme = f.read() requirements = [ + "email_validator", "geoip2>=4.0.0,<5.0.0", "requests>=2.22.0", "rfc3987", "strict-rfc3339", "urllib3>=1.25.2", - "validate_email", "voluptuous", ] From 662cb49406e25f6f25113986a8541699b105bd36 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 11:59:23 -0700 Subject: [PATCH 14/23] Mention dropping Python 2 support and adding type hints in history --- HISTORY.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 82a9ce2..e320c85 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -5,6 +5,9 @@ History 2.0.0 +* IMPORTANT: Python 2.7 and 3.5 support has been dropped. Python 3.6 or greater + is required. +* Type hints have been added. * Email validation is now done with ``email_validator`` rather than ``validate_email``. From cbb4218de99423b80705aca455545ea6afeee23c Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 12:55:23 -0700 Subject: [PATCH 15/23] Switch to urllib.parse for URL validation --- HISTORY.rst | 1 + dev-bin/release.sh | 2 +- minfraud/validation.py | 10 ++++++---- setup.py | 1 - 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index e320c85..38c69be 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -10,6 +10,7 @@ History * Type hints have been added. * Email validation is now done with ``email_validator`` rather than ``validate_email``. +* URL validation is now done with ``urllib.parse`` rather than ``rfc3987``. 1.13.0 (2020-07-14) +++++++++++++++++++ diff --git a/dev-bin/release.sh b/dev-bin/release.sh index e09c5e2..1c72baf 100755 --- a/dev-bin/release.sh +++ b/dev-bin/release.sh @@ -35,7 +35,7 @@ if [ -n "$(git status --porcelain)" ]; then fi # Make sure release deps are installed with the current python -pip install -U sphinx wheel voluptuous strict_rfc3339 email_validator rfc3987 +pip install -U sphinx wheel voluptuous strict_rfc3339 email_validator perl -pi -e "s/(?<=__version__ = \").+?(?=\")/$version/g" minfraud/version.py diff --git a/minfraud/validation.py b/minfraud/validation.py index 560081e..0355b84 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -11,9 +11,9 @@ import ipaddress import re import uuid +import urllib.parse from decimal import Decimal -import rfc3987 from strict_rfc3339 import validate_rfc3339 # I can't reproduce the failure locally and the order looks right to me. @@ -22,6 +22,7 @@ # pylint: disable=wrong-import-order from email_validator import validate_email from voluptuous import All, Any, In, Match, Range, Required, Schema +from voluptuous.error import UrlInvalid from typing import Optional # Pylint doesn't like the private function type naming for the callable @@ -271,9 +272,10 @@ def _rfc3339_datetime(s: str) -> str: def _uri(s: str) -> str: - if rfc3987.parse(s).get("scheme") in ["http", "https"]: - return s - raise ValueError + parsed = urllib.parse.urlparse(s) + if parsed.scheme not in ["http", "https"] or not parsed.netloc: + raise UrlInvalid("URL is invalid") + return s validate_transaction = Schema( diff --git a/setup.py b/setup.py index e61de85..e39e98a 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,6 @@ "email_validator", "geoip2>=4.0.0,<5.0.0", "requests>=2.22.0", - "rfc3987", "strict-rfc3339", "urllib3>=1.25.2", "voluptuous", From 048ad3f0f4be62b589654fa51d297c2cc2bd5989 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 13:15:00 -0700 Subject: [PATCH 16/23] Do RFC3339 validation with regular expression --- HISTORY.rst | 1 + dev-bin/release.sh | 2 +- minfraud/validation.py | 9 +++------ setup.py | 1 - 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 38c69be..3b55958 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -11,6 +11,7 @@ History * Email validation is now done with ``email_validator`` rather than ``validate_email``. * URL validation is now done with ``urllib.parse`` rather than ``rfc3987``. +* RFC 3339 timestamp validation is now done via a regular expression. 1.13.0 (2020-07-14) +++++++++++++++++++ diff --git a/dev-bin/release.sh b/dev-bin/release.sh index 1c72baf..424e11e 100755 --- a/dev-bin/release.sh +++ b/dev-bin/release.sh @@ -35,7 +35,7 @@ if [ -n "$(git status --porcelain)" ]; then fi # Make sure release deps are installed with the current python -pip install -U sphinx wheel voluptuous strict_rfc3339 email_validator +pip install -U sphinx wheel voluptuous email_validator perl -pi -e "s/(?<=__version__ = \").+?(?=\")/$version/g" minfraud/version.py diff --git a/minfraud/validation.py b/minfraud/validation.py index 0355b84..7becf82 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -14,8 +14,6 @@ import urllib.parse from decimal import Decimal -from strict_rfc3339 import validate_rfc3339 - # I can't reproduce the failure locally and the order looks right to me. # It is failing on pylint 1.8.3 on Travis. We should try removing this # when a new version of pylint is released. @@ -246,10 +244,9 @@ def _credit_card_token(s: str) -> str: raise ValueError -def _rfc3339_datetime(s: str) -> str: - if validate_rfc3339(s): - return s - raise ValueError +_rfc3339_datetime = Match( + r"\A\d{4}-\d{2}-\d{2}[Tt]\d{2}:\d{2}:\d{2}(\.\d+)?(?:[Zz]|[+-]\d{2}:\d{2})\Z" +) _event_type = In( diff --git a/setup.py b/setup.py index e39e98a..4ec854e 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,6 @@ "email_validator", "geoip2>=4.0.0,<5.0.0", "requests>=2.22.0", - "strict-rfc3339", "urllib3>=1.25.2", "voluptuous", ] From ab9c1eb7203afb466dd93a03e816fe238bf2bdb5 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 13:25:05 -0700 Subject: [PATCH 17/23] Minor changes to quiet mypy --- minfraud/models.py | 2 +- minfraud/validation.py | 2 +- minfraud/webservice.py | 2 +- tests/test_models.py | 4 +--- tests/test_webservice.py | 2 +- 5 files changed, 5 insertions(+), 7 deletions(-) diff --git a/minfraud/models.py b/minfraud/models.py index 39ca22a..e6c0f10 100644 --- a/minfraud/models.py +++ b/minfraud/models.py @@ -729,7 +729,7 @@ class ServiceWarning: def _create_warnings(warnings: List[Dict[str, str]]) -> Tuple[ServiceWarning, ...]: if not warnings: return () - return tuple([ServiceWarning(x) for x in warnings]) + return tuple([ServiceWarning(x) for x in warnings]) # type: ignore @_inflate_to_namedtuple diff --git a/minfraud/validation.py b/minfraud/validation.py index 7becf82..ab4f62d 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -18,7 +18,7 @@ # It is failing on pylint 1.8.3 on Travis. We should try removing this # when a new version of pylint is released. # pylint: disable=wrong-import-order -from email_validator import validate_email +from email_validator import validate_email # type: ignore from voluptuous import All, Any, In, Match, Range, Required, Schema from voluptuous.error import UrlInvalid from typing import Optional diff --git a/minfraud/webservice.py b/minfraud/webservice.py index 1979d49..cfe210c 100644 --- a/minfraud/webservice.py +++ b/minfraud/webservice.py @@ -218,7 +218,7 @@ def _handle_success( ) if "ip_address" in body: body["ip_address"]["_locales"] = self._locales - return model_class(body) + return model_class(body) # type: ignore def _exception_for_error( self, response: Response, uri: str diff --git a/tests/test_models.py b/tests/test_models.py index c9bdb15..543f714 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,5 +1,3 @@ -import sys - from minfraud.models import * import unittest @@ -25,7 +23,7 @@ def test_model_immutability(self): for model in models: for attr in (model.attr, "does_not_exist"): with self.assertRaises( - AttributeError, msg="{0!s} - {0}".format(model.obj, attr) + AttributeError, msg="{0!s} - {1}".format(model.obj, attr) ): setattr(model.obj, attr, 5) # type: ignore diff --git a/tests/test_webservice.py b/tests/test_webservice.py index 036e511..a847cbb 100644 --- a/tests/test_webservice.py +++ b/tests/test_webservice.py @@ -1,7 +1,7 @@ import os import json -import requests_mock +import requests_mock # type: ignore from io import open from minfraud.errors import ( HTTPError, From 28f048ff89195787b0accdc25ff40856542c2a18 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 13 Jul 2020 13:27:54 -0700 Subject: [PATCH 18/23] Reenable pylint check --- minfraud/validation.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index ab4f62d..3ea63ad 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -13,15 +13,11 @@ import uuid import urllib.parse from decimal import Decimal +from typing import Optional -# I can't reproduce the failure locally and the order looks right to me. -# It is failing on pylint 1.8.3 on Travis. We should try removing this -# when a new version of pylint is released. -# pylint: disable=wrong-import-order from email_validator import validate_email # type: ignore from voluptuous import All, Any, In, Match, Range, Required, Schema from voluptuous.error import UrlInvalid -from typing import Optional # Pylint doesn't like the private function type naming for the callable # objects below. Given the consistent use of them, the current names seem From 9751eefcde2de7f866c0607ade08e7d109d28af5 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 14 Jul 2020 07:35:33 -0700 Subject: [PATCH 19/23] Install geoip2 from Git --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 1b11859..6dbcc8e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,6 +17,7 @@ matrix: before_install: - "if [[ $RUN_SNYK && $SNYK_TOKEN ]]; then sudo apt-get install -y nodejs; npm install -g snyk; fi" install: + - pip install -e git+https://github.com/maxmind/GeoIP2-python#egg=geoip2 - "if [[ $RUN_LINTER ]]; then pip install --upgrade pylint black mypy voluptuous-stubs; fi" script: - python setup.py test From 852ec104f516757a6f11f937e4146aaf947297ae Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 14 Jul 2020 07:53:52 -0700 Subject: [PATCH 20/23] Install twine during release --- dev-bin/release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-bin/release.sh b/dev-bin/release.sh index 424e11e..e5e5293 100755 --- a/dev-bin/release.sh +++ b/dev-bin/release.sh @@ -35,7 +35,7 @@ if [ -n "$(git status --porcelain)" ]; then fi # Make sure release deps are installed with the current python -pip install -U sphinx wheel voluptuous email_validator +pip install -U sphinx wheel voluptuous email_validator twine perl -pi -e "s/(?<=__version__ = \").+?(?=\")/$version/g" minfraud/version.py From 4dacdb1124155386e4efc8d7e07bb3560132ce53 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 14 Jul 2020 08:02:29 -0700 Subject: [PATCH 21/23] Don't use Optional for values we always return --- minfraud/models.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/minfraud/models.py b/minfraud/models.py index e6c0f10..9f91997 100644 --- a/minfraud/models.py +++ b/minfraud/models.py @@ -1049,13 +1049,13 @@ class Factors: billing_address: BillingAddress credit_card: CreditCard disposition: Disposition - funds_remaining: Optional[float] + funds_remaining: float device: Device email: Email - id: Optional[str] + id: str ip_address: IPAddress - queries_remaining: Optional[int] - risk_score: Optional[float] + queries_remaining: int + risk_score: float shipping_address: ShippingAddress subscores: Subscores warnings: List[ServiceWarning] @@ -1176,11 +1176,11 @@ class Insights: device: Device disposition: Disposition email: Email - funds_remaining: Optional[float] - id: Optional[str] + funds_remaining: float + id: str ip_address: IPAddress - queries_remaining: Optional[int] - risk_score: Optional[float] + queries_remaining: int + risk_score: float shipping_address: ShippingAddress warnings: List[ServiceWarning] @@ -1261,11 +1261,11 @@ class Score: """ disposition: Disposition - funds_remaining: Optional[float] - id: Optional[str] + funds_remaining: float + id: str ip_address: ScoreIPAddress - queries_remaining: Optional[int] - risk_score: Optional[float] + queries_remaining: int + risk_score: float warnings: List[ServiceWarning] __slots__ = () From 594db7620b8e78690e91777ed8d2c142bb0d61ac Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 14 Jul 2020 14:00:40 -0700 Subject: [PATCH 22/23] Set the ASCII flag on the regex --- minfraud/validation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/minfraud/validation.py b/minfraud/validation.py index 3ea63ad..8f78477 100644 --- a/minfraud/validation.py +++ b/minfraud/validation.py @@ -241,7 +241,7 @@ def _credit_card_token(s: str) -> str: _rfc3339_datetime = Match( - r"\A\d{4}-\d{2}-\d{2}[Tt]\d{2}:\d{2}:\d{2}(\.\d+)?(?:[Zz]|[+-]\d{2}:\d{2})\Z" + r"(?a)\A\d{4}-\d{2}-\d{2}[Tt]\d{2}:\d{2}:\d{2}(\.\d+)?(?:[Zz]|[+-]\d{2}:\d{2})\Z" ) From 190ee89aab872a2340258520080a0e56f4f87797 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 14 Jul 2020 14:02:29 -0700 Subject: [PATCH 23/23] Remove specific service names from Client summary --- minfraud/webservice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/minfraud/webservice.py b/minfraud/webservice.py index cfe210c..bea37dd 100644 --- a/minfraud/webservice.py +++ b/minfraud/webservice.py @@ -27,7 +27,7 @@ class Client: - """Client for accessing the minFraud Score and Insights web services.""" + """Client for accessing the minFraud web services.""" def __init__( # pylint: disable=too-many-arguments self,