diff --git a/README.rst b/README.rst index c586959..da512b1 100644 --- a/README.rst +++ b/README.rst @@ -44,31 +44,34 @@ Add ``'adyen'`` to ``INSTALLED_APPS`` and run:: to create the appropriate database tables. Configuration -------------- +============= -Edit your ``settings.py`` to set the following settings: - -.. code-block:: python +You have two approaches to configure `django-oscar-adyen`. - ADYEN_IDENTIFIER = 'YourAdyenAccountName' - ADYEN_SECRET_KEY = 'YourAdyenSkinSecretKey' - ADYEN_ACTION_URL = 'https://test.adyen.com/hpp/select.shtml' +Settings-based configuration +---------------------------- +For simple deployments, setting the required values in the settings will suffice. -Obviously, you'll need to specify different settings in your test environment -as opposed to your production environment. +Edit your ``settings.py`` to set the following settings: +* ``ADYEN_IDENTIFIER`` - The identifier of your Adyen account. +* ``ADYEN_SKIN_CODE`` - The code for your Adyen skin. +* ``ADYEN_SECRET_KEY`` - The secret key defined in your Adyen skin. +* ``ADYEN_ACTION_URL`` - + The URL towards which the Adyen form should be POSTed to initiate the payment process + (e.g. 'https://test.adyen.com/hpp/select.shtml'). +* ``ADYEN_IP_ADDRESS_HTTP_HEADER`` - Optional. The header in `META` to inspect to determine + the IP address of the request. Defaults to `REMOTE_ADDR`. -Settings -======== +You will likely need to specify different settings in your test environment +as opposed to your production environment. -====================== ========================================================= - Setting Description ----------------------- --------------------------------------------------------- - ``ADYEN_IDENTIFIER`` The identifier of your Adyen account - ``ADYEN_SECRET_KEY`` The secret key defined in your Adyen skin - ``ADYEN_ACTION_URL`` The URL towards which the Adyen form should be POSTed - to initiate the payment process -====================== ========================================================= +Class-based configuration +------------------------- +In more complex deployments, you will want to e.g. alter the Adyen identifier based on +the request. That is not easily implemented with Django settings, so you can alternatively +set ``ADYEN_CONFIG_CLASS`` to a config class of your own. +See `adyen.settings_config.FromSettingsConfig` for an example. License ======= diff --git a/adyen/config.py b/adyen/config.py new file mode 100644 index 0000000..36607c9 --- /dev/null +++ b/adyen/config.py @@ -0,0 +1,35 @@ +from django.conf import settings +from django.utils.module_loading import import_string + + +def get_config(): + """ + Returns an instance of the configured config class. + """ + + try: + config_class_string = settings.ADYEN_CONFIG_CLASS + except AttributeError: + config_class_string = 'adyen.settings_config.FromSettingsConfig' + return import_string(config_class_string)() + + +class AbstractAdyenConfig: + """ + The base implementation for a config class. + """ + + def get_identifier(self, request): + raise NotImplementedError + + def get_action_url(self, request): + raise NotImplementedError + + def get_skin_code(self, request): + raise NotImplementedError + + def get_skin_secret(self, request): + raise NotImplementedError + + def get_ip_address_header(self): + raise NotImplementedError diff --git a/adyen/facade.py b/adyen/facade.py index 0db0f7b..99f805c 100644 --- a/adyen/facade.py +++ b/adyen/facade.py @@ -3,32 +3,33 @@ import iptools import logging -from django.conf import settings from django.http import HttpResponse from .gateway import Constants, Gateway, PaymentNotification, PaymentRedirection from .models import AdyenTransaction +from .config import get_config logger = logging.getLogger('adyen') +def get_gateway(request, config): + return Gateway({ + Constants.IDENTIFIER: config.get_identifier(request), + Constants.SECRET_KEY: config.get_skin_secret(request), + Constants.ACTION_URL: config.get_action_url(request), + }) -class Facade(): - def __init__(self, **kwargs): - init_params = { - Constants.IDENTIFIER: settings.ADYEN_IDENTIFIER, - Constants.SECRET_KEY: settings.ADYEN_SECRET_KEY, - Constants.ACTION_URL: settings.ADYEN_ACTION_URL, - } - # Initialize the gateway. - self.gateway = Gateway(init_params) +class Facade: + + def __init__(self): + self.config = get_config() - def build_payment_form_fields(self, params): + def build_payment_form_fields(self, request, params): """ Return a dict containing the name and value of all the hidden fields necessary to build the form that will be POSTed to Adyen. """ - return self.gateway.build_payment_form_fields(params) + return get_gateway(request, self.config).build_payment_form_fields(params) @classmethod def _is_valid_ip_address(cls, s): @@ -39,8 +40,7 @@ def _is_valid_ip_address(cls, s): """ return iptools.ipv4.validate_ip(s) or iptools.ipv6.validate_ip(s) - @classmethod - def _get_origin_ip_address(cls, request): + def _get_origin_ip_address(self, request): """ Return the IP address where the payment originated from or None if we are unable to get it -- which *will* happen if we received a @@ -54,17 +54,14 @@ def _get_origin_ip_address(cls, request): Django setting. We fallback on the canonical `REMOTE_ADDR`, used for regular, unproxied requests. """ - try: - ip_address_http_header = settings.ADYEN_IP_ADDRESS_HTTP_HEADER - except AttributeError: - ip_address_http_header = 'REMOTE_ADDR' + ip_address_http_header = self.config.get_ip_address_header() try: ip_address = request.META[ip_address_http_header] except KeyError: return None - if not cls._is_valid_ip_address(ip_address): + if not self._is_valid_ip_address(ip_address): logger.warn("%s is not a valid IP address", ip_address) return None @@ -146,11 +143,7 @@ def handle_payment_feedback(self, request, record_audit_trail): Validate, process, optionally record audit trail and provide feedback about the current payment response. """ - success, output_data = False, {} - # We must first find out whether this is a redirection or a notification. - client = self.gateway - params = response_class = None if request.method == 'GET': params = request.GET @@ -162,7 +155,8 @@ def handle_payment_feedback(self, request, record_audit_trail): raise RuntimeError("Only GET and POST requests are supported.") # Then we can instantiate the appropriate class from the gateway. - response = response_class(client, params) + gateway = get_gateway(request, self.config) + response = response_class(gateway, params) # Note that this may raise an exception if the response is invalid. # For example: MissingFieldException, UnexpectedFieldException, ... @@ -209,7 +203,7 @@ def assess_notification_relevance(self, request): # - On the other hand we have the `live` POST parameter, which lets # us know which Adyen platform fired this request. current_platform = (Constants.LIVE - if Constants.LIVE in settings.ADYEN_ACTION_URL + if Constants.LIVE in self.config.get_action_url(request) else Constants.TEST) origin_platform = (Constants.LIVE diff --git a/adyen/scaffold.py b/adyen/scaffold.py index 855672c..874367c 100644 --- a/adyen/scaffold.py +++ b/adyen/scaffold.py @@ -1,16 +1,11 @@ -# -*- coding: utf-8 -*- - -import bleach - -from django.conf import settings -from django.core.exceptions import ImproperlyConfigured from django.utils import timezone from .facade import Facade from .gateway import Constants, MissingFieldException +from .config import get_config -class Scaffold(): +class Scaffold: # These are the constants that all scaffolds are expected to return # to a multi-psp application. They might look like those actually returned @@ -26,34 +21,17 @@ class Scaffold(): Constants.PAYMENT_RESULT_REFUSED: PAYMENT_STATUS_REFUSED, } - def __init__(self, order_data=None): - self.facade = Facade() - try: - for name, value in order_data.items(): - setattr(self, name, value) - except AttributeError: - pass + def __init__(self): + self.config = get_config() - def get_form_action(self): + def get_form_action(self, request): """ Return the URL where the payment form should be submitted. """ - try: - return settings.ADYEN_ACTION_URL - except AttributeError: - raise ImproperlyConfigured("Please set ADYEN_ACTION_URL") + return self.config.get_action_url(request) - def get_form_fields(self): - """ Return the payment form fields, rendered into HTML. """ - - fields_list = self.get_form_fields_list() - return ''.join([ - '\n' % ( - f.get('type'), f.get('name'), bleach.clean(f.get('value')) - ) for f in fields_list - ]) - - def get_form_fields_list(self): + def get_form_fields(self, request, order_data): """ Return the payment form fields as a list of dicts. + Expects a large-ish order_data dictionary with details of the order. """ now = timezone.now() session_validity = now + timezone.timedelta(minutes=20) @@ -64,35 +42,33 @@ def get_form_fields_list(self): # Build common field specs try: field_specs = { - Constants.MERCHANT_ACCOUNT: settings.ADYEN_IDENTIFIER, - Constants.MERCHANT_REFERENCE: str(self.order_number), - Constants.SHOPPER_REFERENCE: self.client_id, - Constants.SHOPPER_EMAIL: self.client_email, - Constants.CURRENCY_CODE: self.currency_code, - Constants.PAYMENT_AMOUNT: self.amount, - Constants.SKIN_CODE: settings.ADYEN_SKIN_CODE, + Constants.MERCHANT_ACCOUNT: self.config.get_identifier(request), + Constants.SKIN_CODE: self.config.get_skin_code(request), Constants.SESSION_VALIDITY: session_validity.strftime(session_validity_format), Constants.SHIP_BEFORE_DATE: ship_before_date.strftime(ship_before_date_format), - Constants.SHOPPER_LOCALE: self.shopper_locale, - Constants.COUNTRY_CODE: self.country_code, - - # Adyen does not provide the payment amount in the - # return URL, so we store it in this field to - # avoid a database query to get it back then. - Constants.MERCHANT_RETURN_DATA: self.amount, + Constants.MERCHANT_REFERENCE: str(order_data['order_number']), + Constants.SHOPPER_REFERENCE: order_data['client_id'], + Constants.SHOPPER_EMAIL: order_data['client_email'], + Constants.CURRENCY_CODE: order_data['currency_code'], + Constants.PAYMENT_AMOUNT: order_data['amount'], + Constants.SHOPPER_LOCALE: order_data['shopper_locale'], + Constants.COUNTRY_CODE: order_data['country_code'], + # Adyen does not provide the payment amount in the return URL, so we store it in + # this field to avoid a database query to get it back then. + Constants.MERCHANT_RETURN_DATA: order_data['amount'], } - except AttributeError: - raise MissingFieldException + except KeyError: + raise MissingFieldException("One or more fields are missing from the order data.") # Check for overridden return URL. - return_url = getattr(self, 'return_url', None) + return_url = order_data.get('return_url', None) if return_url is not None: return_url = return_url.replace('PAYMENT_PROVIDER_CODE', Constants.ADYEN) field_specs[Constants.MERCHANT_RETURN_URL] = return_url - return self.facade.build_payment_form_fields(field_specs) + return Facade().build_payment_form_fields(request, field_specs) def _normalize_feedback(self, feedback): """ @@ -105,16 +81,14 @@ def _normalize_feedback(self, feedback): def handle_payment_feedback(self, request): return self._normalize_feedback( - self.facade.handle_payment_feedback( - request, record_audit_trail=True)) + Facade().handle_payment_feedback(request, record_audit_trail=True)) def check_payment_outcome(self, request): return self._normalize_feedback( - self.facade.handle_payment_feedback( - request, record_audit_trail=False)) + Facade().handle_payment_feedback(request, record_audit_trail=False)) def assess_notification_relevance(self, request): - return self.facade.assess_notification_relevance(request) + return Facade().assess_notification_relevance(request) def build_notification_response(self, request): - return self.facade.build_notification_response(request) + return Facade().build_notification_response(request) diff --git a/adyen/settings_config.py b/adyen/settings_config.py new file mode 100644 index 0000000..21fa7fd --- /dev/null +++ b/adyen/settings_config.py @@ -0,0 +1,43 @@ +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured + +from .config import AbstractAdyenConfig + + +class FromSettingsConfig(AbstractAdyenConfig): + """ + This config class is enabled by default and useful in simple deployments. + One can just set all needed values in the Django settings. It also + exists for backwards-compatibility with previous deployments. + """ + + def __init__(self): + """ + We complain as early as possible when Django settings are missing. + """ + required_settings = [ + 'ADYEN_IDENTIFIER', 'ADYEN_ACTION_URL', 'ADYEN_SKIN_CODE', 'ADYEN_SECRET_KEY'] + missing_settings = [ + setting for setting in required_settings if not hasattr(settings, setting)] + if missing_settings: + raise ImproperlyConfigured( + "You are using the FromSettingsConfig config class, but haven't set the " + "the following required settings: %s" % missing_settings) + + def get_identifier(self, request): + return settings.ADYEN_IDENTIFIER + + def get_action_url(self, request): + return settings.ADYEN_ACTION_URL + + def get_skin_code(self, request): + return settings.ADYEN_SKIN_CODE + + def get_skin_secret(self, request): + return settings.ADYEN_SECRET_KEY + + def get_ip_address_header(self): + try: + return settings.ADYEN_IP_ADDRESS_HTTP_HEADER + except AttributeError: + return 'REMOTE_ADDR' diff --git a/requirements.txt b/requirements.txt index 3c42042..60c3c2b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,3 @@ -bleach==1.4 iptools==0.6.1 requests>=2.0.0,<3.0 freezegun==0.1.18 diff --git a/setup.py b/setup.py index c87d3e7..ac593a1 100644 --- a/setup.py +++ b/setup.py @@ -25,7 +25,6 @@ packages=find_packages(), include_package_data=True, install_requires=[ - 'bleach==1.4', 'django-oscar>=0.7', 'iptools==0.6.1', 'requests>=2.0,<3.0', diff --git a/tests/test_adyen.py b/tests/test_adyen.py index f124baf..e1dd3bf 100644 --- a/tests/test_adyen.py +++ b/tests/test_adyen.py @@ -4,8 +4,6 @@ import six from unittest.mock import Mock -from django.conf import settings -from django.core.exceptions import ImproperlyConfigured from django.test import TestCase from django.test.utils import override_settings @@ -102,6 +100,22 @@ 'skinCode': '4d72uQqA', } +ORDER_DATA = { + 'amount': 123, + 'basket_id': 456, + 'client_email': 'test@example.com', + 'client_id': 789, + 'currency_code': 'EUR', + 'country_code': 'fr', + 'description': 'Order #123', + 'order_id': 'ORD-123', + 'order_number': '00000000123', + 'return_url': TEST_RETURN_URL, + 'shopper_locale': 'fr', +} + +DUMMY_REQUEST = None + @override_settings( ADYEN_IDENTIFIER=TEST_IDENTIFIER, @@ -113,21 +127,7 @@ class AdyenTestCase(TestCase): def setUp(self): super().setUp() - - self.order_data = { - 'amount': 123, - 'basket_id': 456, - 'client_email': 'test@example.com', - 'client_id': 789, - 'currency_code': 'EUR', - 'country_code': 'fr', - 'description': 'Order #123', - 'order_id': 'ORD-123', - 'order_number': '00000000123', - 'return_url': TEST_RETURN_URL, - 'shopper_locale': 'fr', - } - self.scaffold = Scaffold(self.order_data) + self.scaffold = Scaffold() class TestAdyenPaymentRequest(AdyenTestCase): @@ -137,47 +137,29 @@ def test_form_action(self): """ Test that the form action is properly fetched from the settings. """ - action_url = self.scaffold.get_form_action() + action_url = self.scaffold.get_form_action(DUMMY_REQUEST) self.assertEqual(action_url, TEST_ACTION_URL) - # If the setting is missing, a proper exception is raised - del settings.ADYEN_ACTION_URL - with self.assertRaises(ImproperlyConfigured): - self.scaffold.get_form_action() - def test_form_fields_ok(self): - """ - Test that the payment form fields are properly built. - """ - with freeze_time(TEST_FROZEN_TIME): - form_fields = self.scaffold.get_form_fields() - for field_spec in EXPECTED_FIELDS_LIST: - field = '' % ( - field_spec.get('type'), - field_spec.get('name'), - field_spec.get('value'), - ) - self.assertIn(field, form_fields) - - def test_form_fields_list_ok(self): """ Test that the payment form fields list is properly built. """ with freeze_time(TEST_FROZEN_TIME): - fields_list = self.scaffold.get_form_fields_list() + fields_list = self.scaffold.get_form_fields(DUMMY_REQUEST, ORDER_DATA) self.assertEqual(len(fields_list), len(EXPECTED_FIELDS_LIST)) for field in fields_list: self.assertIn(field, EXPECTED_FIELDS_LIST) - def test_form_fields_list_with_missing_mandatory_field(self): + def test_form_fields_with_missing_mandatory_field(self): """ Test that the proper exception is raised when trying to build a fields list with a missing mandatory field. """ - del self.order_data['amount'] - scaffold = Scaffold(self.order_data) + new_order_data = ORDER_DATA.copy() + del new_order_data['amount'] + with self.assertRaises(MissingFieldException): - scaffold.get_form_fields_list() + self.scaffold.get_form_fields(DUMMY_REQUEST, new_order_data) class TestAdyenPaymentResponse(AdyenTestCase): @@ -229,11 +211,12 @@ def test_get_origin_ip_address(self): the possible meaningful combinations of default and custom HTTP header names. """ + facade = Facade() # With no specified ADYEN_IP_ADDRESS_HTTP_HEADER setting, # ensure we fetch the origin IP address in the REMOTE_ADDR # HTTP header. - ip_address = Facade._get_origin_ip_address(self.request) + ip_address = facade._get_origin_ip_address(self.request) self.assertEqual(ip_address, '127.0.0.1') if six.PY3: self.assertEqual(type(ip_address), str) @@ -241,16 +224,17 @@ def test_get_origin_ip_address(self): # Check the return value is None if we have nothing # in the `REMOTE_ADDR` header. self.request.META.update({'REMOTE_ADDR': ''}) - ip_address = Facade._get_origin_ip_address(self.request) + ip_address = facade._get_origin_ip_address(self.request) self.assertIsNone(ip_address) # Check the return value is None if we have no `REMOTE_ADDR` # header at all. del self.request.META['REMOTE_ADDR'] - ip_address = Facade._get_origin_ip_address(self.request) + ip_address = facade._get_origin_ip_address(self.request) self.assertIsNone(ip_address) with self.settings(ADYEN_IP_ADDRESS_HTTP_HEADER=TEST_IP_ADDRESS_HTTP_HEADER): + facade = Facade() # Recreate the instance to update the Adyen config. # Now we add the `HTTP_X_FORWARDED_FOR` header and # ensure it is used instead. @@ -258,21 +242,21 @@ def test_get_origin_ip_address(self): 'REMOTE_ADDR': '127.0.0.1', 'HTTP_X_FORWARDED_FOR': '93.16.93.168' }) - ip_address = Facade._get_origin_ip_address(self.request) + ip_address = facade._get_origin_ip_address(self.request) self.assertEqual(ip_address, '93.16.93.168') if six.PY3: self.assertEqual(type(ip_address), str) # Even if the default header is missing. del self.request.META['REMOTE_ADDR'] - ip_address = Facade._get_origin_ip_address(self.request) + ip_address = facade._get_origin_ip_address(self.request) self.assertEqual(ip_address, '93.16.93.168') if six.PY3: self.assertEqual(type(ip_address), str) # And finally back to `None` if we have neither header. del self.request.META['HTTP_X_FORWARDED_FOR'] - ip_address = Facade._get_origin_ip_address(self.request) + ip_address = facade._get_origin_ip_address(self.request) self.assertIsNone(ip_address) def test_handle_authorised_payment(self): @@ -363,7 +347,7 @@ def test_handle_authorized_payment_if_no_ip_address_was_found(self): del self.request.META['REMOTE_ADDR'] # ... double-check that the IP address is, therefore, `None` ... - ip_address = Facade._get_origin_ip_address(self.request) + ip_address = Facade()._get_origin_ip_address(self.request) self.assertIsNone(ip_address) # ... and finally make sure everything works as expected. diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..96d47a8 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,64 @@ +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured + +from django.test import TestCase +from django.test.utils import override_settings + +# We use get_config() instead of adyen_config because throughout +# the tests, we repeatedly change the Django settings. +from adyen.config import get_config, AbstractAdyenConfig +from adyen.settings_config import FromSettingsConfig + + +@override_settings( + ADYEN_IDENTIFIER='foo', + ADYEN_SECRET_KEY='foo', + ADYEN_ACTION_URL='foo', + ADYEN_SKIN_CODE='foo', +) +class FromSettingsTestCase(TestCase): + """ + This test case tests the FromSettings config class, which just fetches its + values from the Django settings. + """ + + def test_is_default(self): + assert isinstance(get_config(), FromSettingsConfig) + + def test_value_passing_works(self): + assert get_config().get_action_url(None) == 'foo' + + # https://docs.djangoproject.com/en/1.8/topics/testing/tools/#django.test.modify_settings + # Override settings is needed to let us delete settings on a per-test basis. + @override_settings() + def test_complains_when_not_fully_configured(self): + # If the setting is missing, a proper exception is raised + del settings.ADYEN_ACTION_URL + with self.assertRaises(ImproperlyConfigured): + get_config() + + +class DummyConfigClass(AbstractAdyenConfig): + + def get_action_url(self, request): + return 'foo' + + +@override_settings(ADYEN_CONFIG_CLASS='tests.test_config.DummyConfigClass') +class CustomConfigClassTestCase(TestCase): + """ + This test case checks that it's possible to replace the FromSettings confic class + by one's own, and that it is used to fetch values as expected. + """ + + def test_class_gets_picked_up(self): + assert isinstance(get_config(), DummyConfigClass) + + @override_settings(ADYEN_ACTION_URL='bar') + def test_settings_ignored(self): + """ + Check that we indeed ignore Django settings (apart from the config class). + """ + assert get_config().get_action_url(None) == 'foo' + +