From a723d115faf7910a47dedbecfdbc6d15081dd7b9 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 21 Jun 2024 15:15:31 +0200 Subject: [PATCH 1/3] Catch 503 error from Akamai, which is an upstream provider for MailChimp. You get this when you are (temporarily) blocked. --- CHANGES.rst | 6 +++++ src/collective/mailchimp/exceptions.py | 20 +++++++++++++++ src/collective/mailchimp/locator.py | 35 ++++++++++++++++++++++++-- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6dbd213..3bb8981 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,6 +12,12 @@ New features: - Test on Plone 6.0 as well, next to Plone 5.2. [maurits] +Bug fixes: + +- Catch 503 error from Akamai, which is an upstream provider for MailChimp. + You get this when you are (temporarily) blocked. + [maurits] + 4.0.0a1 (2023-04-14) -------------------- diff --git a/src/collective/mailchimp/exceptions.py b/src/collective/mailchimp/exceptions.py index 1747ce7..1bb4243 100644 --- a/src/collective/mailchimp/exceptions.py +++ b/src/collective/mailchimp/exceptions.py @@ -56,3 +56,23 @@ def __str__(self): return 'MailChimp error code {0}: "{1}"\n{2}'.format( self.code, self.detail, self.errors ) + + +class AkamaiException(MailChimpException): + """Akamai is an provider upstream for MailChimp. + + It may give 503 code if you are blocked. Sample: + + {'type': 'akamai_error_message', 'title': 'akamai_503', 'status': 503, + 'ref_no': 'Reference Number: ...'} + """ + + def __init__(self, code, detail, errors=''): + self.code = code + self.detail = detail + self.errors = errors + + def __str__(self): + return 'Akamai error code {0}: "{1}"\n{2}'.format( + self.code, self.detail, self.errors + ) diff --git a/src/collective/mailchimp/locator.py b/src/collective/mailchimp/locator.py index 98b7318..e3414e9 100644 --- a/src/collective/mailchimp/locator.py +++ b/src/collective/mailchimp/locator.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +from .exceptions import AkamaiException from .exceptions import DeserializationError from .exceptions import MailChimpException from .exceptions import PostRequestError @@ -114,12 +115,42 @@ def _fail_if_mailchimp_exc(self, response): if not isinstance(response, dict): return # case: response is a dict and may be an error response - elif 'status' in response and 'detail' in response: + # We need a status. + if 'status' not in response: + return + status = response['status'] + if 'detail' in response: exc = MailChimpException( - response['status'], response['detail'], response.get('errors') + status, response['detail'], response.get('errors') ) logger.warn(exc) raise exc + # Is status a number? + try: + status = int(status) + except ValueError: + return + if status >= 400: + # Maybe we should do this from 300 onwards, but that would include + # redirects, which might be fine. In my case I have seen 503, + # which apparently means we are blocked. Hopefully temporarily. + # {'type': 'akamai_error_message', 'title': 'akamai_503', 'status': 503, + # 'ref_no': 'Reference Number: ...'} + if ( + response.get('type', '') == 'akamai_error_message' + and 'ref_no' in response + ): + exc = AkamaiException( + status, response['ref_no'], response.get('title') + ) + else: + # Log the full response, as we cannot really parse it. + logger.warn("Can't properly parse response: %r", response) + exc = MailChimpException( + status, 'no details available', response.get('errors') + ) + logger.warn(exc) + raise exc def api_request(self, endpoint='', request_type='get', query_params={}, **kwargs): """ construct the request and do a get/post. From b920d3e3a1c5718fefe01e68d672ad9b32331162 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 21 Jun 2024 15:50:38 +0200 Subject: [PATCH 2/3] Raise BadRequest error when the list id is illegal. This tries to avoid hacking attempts that can get you banned. --- CHANGES.rst | 4 +++ .../mailchimp/browser/newsletter.py | 30 +++++++++++++++++-- src/collective/mailchimp/tests/test_unit.py | 19 ++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 src/collective/mailchimp/tests/test_unit.py diff --git a/CHANGES.rst b/CHANGES.rst index 3bb8981..5c6db5d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -14,6 +14,10 @@ New features: Bug fixes: +- Raise BadRequest error when the list id is illegal. + This tries to avoid hacking attempts that can get you banned. + [maurits] + - Catch 503 error from Akamai, which is an upstream provider for MailChimp. You get this when you are (temporarily) blocked. [maurits] diff --git a/src/collective/mailchimp/browser/newsletter.py b/src/collective/mailchimp/browser/newsletter.py index 3e93ff0..70c0a32 100644 --- a/src/collective/mailchimp/browser/newsletter.py +++ b/src/collective/mailchimp/browser/newsletter.py @@ -20,11 +20,19 @@ from z3c.form.interfaces import ActionExecutionError from z3c.form.interfaces import HIDDEN_MODE from z3c.form.interfaces import WidgetActionExecutionError +from zExceptions import BadRequest from zope.annotation.interfaces import IAttributeAnnotatable from zope.component import getUtility from zope.interface import implementer from zope.interface import Invalid +import re + + +# Characters that are allowed in some fields. +# This tries to prevent hacking attempts that get us blocked. +CHARS_ALLOWED = re.compile(r"^[a-zA-Z0-9\-_\./@\+]*$").match + @implementer(INewsletterSubscribe, IAttributeAnnotatable) class NewsletterSubcriber(object): @@ -93,7 +101,15 @@ def handleApply(self, action): # Retrieve list_id either from a hidden field in the form or fetch # the first list from mailchimp. - list_id = data.get('list_id') or self.mailchimp.default_list_id() + list_id = data.get('list_id') + if list_id and not CHARS_ALLOWED(list_id): + # I saw a hacker adapt the hidden field: + # "actual-id' UNION ALL SELECT NULL,NULL,NULL,NULL#" + # Currently the list id is hexadecimal, but I suppose + # we can't rely on that. + raise BadRequest("list_id has disallowed characters") + + list_id = list_id or self.mailchimp.default_list_id() # list_id has to be updated for merge_fields=data to work if None if "list_id" in data and not data.get('list_id'): data['list_id'] = list_id @@ -102,12 +118,20 @@ def handleApply(self, action): interests = {} interest_groups = data.pop('interest_groups', []) if self.available_interest_groups and interest_groups: + interest_groups = map(safe_text, interest_groups) + for group in interest_groups: + if not CHARS_ALLOWED(group): + raise BadRequest("interest_groups has disallowed characters") # Create dictionary with as keys the interest groups, and as # values always True. - interests = dict.fromkeys(map(safe_text, interest_groups), True) + interests = dict.fromkeys(interest_groups, True) # Use email_type if one is provided by the form, if not choose the # default email type from the control panel settings. - email_type = data.get('email_type') or self.mailchimp_settings.email_type + email_type = data.get('email_type') + if email_type and not CHARS_ALLOWED(email_type): + raise BadRequest("email_type has disallowed characters") + + email_type = email_type or self.mailchimp_settings.email_type # Subscribe to MailChimp list try: diff --git a/src/collective/mailchimp/tests/test_unit.py b/src/collective/mailchimp/tests/test_unit.py new file mode 100644 index 0000000..43c5dfd --- /dev/null +++ b/src/collective/mailchimp/tests/test_unit.py @@ -0,0 +1,19 @@ + +import unittest + + +class TestUnit(unittest.TestCase): + def test_list_id_allowed(self): + from collective.mailchimp.browser.newsletter import CHARS_ALLOWED as allowed + + self.assertTrue(allowed("abcde12345")) + self.assertTrue(allowed("12345ABCDE")) + self.assertTrue(allowed("4f17c3b08a")) + self.assertTrue(allowed("html")) + self.assertTrue(allowed("plain-text")) + self.assertTrue(allowed("text/plain")) + self.assertTrue(allowed("maurits@example.org")) + self.assertTrue(allowed("maurits+test@example.org")) + self.assertFalse(allowed("abcde1234' hack em")) + self.assertFalse(allowed("abcde1234;")) + self.assertFalse(allowed("abcde1234?")) From 08f754f012d0670ce7536a7bb70e9b72db16e9f1 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 21 Jun 2024 23:11:25 +0200 Subject: [PATCH 3/3] Handle all exceptions when subscribing or unsubscribing. Otherwise you will keep seeing the form and never see an error. --- CHANGES.rst | 4 ++++ src/collective/mailchimp/browser/newsletter.py | 13 +++++++------ src/collective/mailchimp/locator.py | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 5c6db5d..d4f21ec 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -14,6 +14,10 @@ New features: Bug fixes: +- Handle all exceptions when subscribing or unsubscribing. + Otherwise you will keep seeing the form and never see an error. + [maurits] + - Raise BadRequest error when the list id is illegal. This tries to avoid hacking attempts that can get you banned. [maurits] diff --git a/src/collective/mailchimp/browser/newsletter.py b/src/collective/mailchimp/browser/newsletter.py index 70c0a32..458e7f6 100644 --- a/src/collective/mailchimp/browser/newsletter.py +++ b/src/collective/mailchimp/browser/newsletter.py @@ -1,6 +1,5 @@ # -*- coding: utf-8 -*- from collective.mailchimp import _ -from collective.mailchimp.exceptions import MailChimpException from collective.mailchimp.interfaces import IMailchimpLocator from collective.mailchimp.interfaces import IMailchimpSettings from collective.mailchimp.interfaces import INewsletterSubscribe @@ -142,7 +141,7 @@ def handleApply(self, action): interests=interests, merge_fields=data, ) - except MailChimpException as error: + except Exception as error: return self.handle_error(error, data) if self.mailchimp_settings.double_optin: @@ -166,7 +165,9 @@ def handleApply(self, action): def handle_error(self, error, data): # Current api v3 documentation only lists errors in the 400 and 500 # range. The 400 code can mean a lot of things... - if error.code == 400: + # Also, not all exceptions have a code. + error_code = getattr(error, "code", 500) + if error_code == 400: error_msg = _( u"mailchimp_error_msg_already_subscribed", default=u"Could not subscribe to newsletter. " @@ -178,7 +179,7 @@ def handle_error(self, error, data): raise WidgetActionExecutionError( 'email', Invalid(translated_error_msg) ) - elif error.code == 220: + elif error_code == 220: error_msg = _( u"mailchimp_error_msg_banned", default=u"Could not subscribe to newsletter. " @@ -272,8 +273,8 @@ def handle_unsubscribe(self, action): self.mailchimp.update_subscriber( list_id, email_address=email, **update_data ) - except MailChimpException as error: - if error.code != 404: + except Exception as error: + if getattr(error, "code", 500) != 404: # If a subscriber did not exist we don't want to announce # it. Treat only != 404 as an error. IStatusMessage(self.request).addStatusMessage( diff --git a/src/collective/mailchimp/locator.py b/src/collective/mailchimp/locator.py index e3414e9..72e04a2 100644 --- a/src/collective/mailchimp/locator.py +++ b/src/collective/mailchimp/locator.py @@ -288,7 +288,7 @@ def subscribe(self, list_id, email_address, email_type, **kwargs): except MailChimpException: raise except Exception as e: - raise PostRequestError(e) + raise PostRequestError(e) logger.info( "Subscribed %s to list with id: %s." % (email_address, list_id) )