fix(client): security hardening, input validation for request paths

README.md

@@ -203,6 +203,11 @@ except CheckoutApiException as err:
     error_details = err.error_details
 ```
 
+Identifiers passed to client methods (for example a payment id) are sent as single, percent-encoded path
+segments: pass them unencoded (Enum members are sent by their value). A `CheckoutArgumentException` is raised
+before any request is sent when an identifier is not a string, is `None`, empty, `.` or `..`, cannot be encoded as UTF-8, or contains `/`, `\` or an
+ASCII control character (U+0000 to U+001F, U+007F).
+
 ## Building from source
 
 ### Prerequisites

checkout_sdk/apm/sepa_client.py

@@ -8,7 +8,8 @@
 
 class SepaClient(Client):
     __APMS_PATH = 'apms'
-    __SEPA_MANDATES_PATH = 'sepa/mandates'
+    __SEPA_PATH = 'sepa'
+    __MANDATES_PATH = 'mandates'
     __PPRO_PATH = 'ppro'
     __CANCEL_PATH = 'cancel'
 
@@ -18,18 +19,21 @@ def __init__(self, api_client: ApiClient, configuration: CheckoutConfiguration):
                          authorization_type=AuthorizationType.SECRET_KEY)
 
     def get_mandate(self, mandate_id: str):
-        return self._api_client.get(self.build_path(self.__SEPA_MANDATES_PATH, mandate_id), self._sdk_authorization())
+        return self._api_client.get(self.build_path(self.__SEPA_PATH, self.__MANDATES_PATH, mandate_id),
+                                    self._sdk_authorization())
 
     def cancel_mandate(self, mandate_id: str):
-        return self._api_client.post(self.build_path(self.__SEPA_MANDATES_PATH, mandate_id, self.__CANCEL_PATH),
+        return self._api_client.post(self.build_path(self.__SEPA_PATH, self.__MANDATES_PATH,
+                                                     mandate_id, self.__CANCEL_PATH),
                                      self._sdk_authorization())
 
     def get_mandate_via_ppro(self, mandate_id: str):
         return self._api_client.get(self.build_path(self.__APMS_PATH, self.__PPRO_PATH,
-                                                    self.__SEPA_MANDATES_PATH, mandate_id),
+                                                    self.__SEPA_PATH, self.__MANDATES_PATH, mandate_id),
                                     self._sdk_authorization())
 
     def cancel_mandate_via_ppro(self, mandate_id: str):
         return self._api_client.post(self.build_path(self.__APMS_PATH, self.__PPRO_PATH,
-                                                     self.__SEPA_MANDATES_PATH, mandate_id, self.__CANCEL_PATH),
+                                                     self.__SEPA_PATH, self.__MANDATES_PATH,
+                                                     mandate_id, self.__CANCEL_PATH),
                                      self._sdk_authorization())

checkout_sdk/client.py

@@ -1,8 +1,17 @@
 from __future__ import absolute_import
 
+import re
+from enum import Enum
+from urllib.parse import quote
+
 from checkout_sdk.api_client import ApiClient
 from checkout_sdk.authorization_type import AuthorizationType
 from checkout_sdk.checkout_configuration import CheckoutConfiguration
+from checkout_sdk.exception import CheckoutArgumentException
+
+_FORBIDDEN_SEGMENT_CHARACTERS = re.compile(r'[/\\\x00-\x1f\x7f]')
+_INVALID_SEGMENT_MESSAGE = ("Invalid path segment: it must be a non-empty string, not '.' or '..', "
+                            "without '/', '\\' or ASCII control characters (U+0000 to U+001F, U+007F)")
 
 
 class Client:
@@ -25,4 +34,41 @@ def is_sandbox(self):
 
     @staticmethod
     def build_path(*params):
-        return '/'.join(params)
+        """Join path segments into a request path, encoding each one as a single segment.
+
+        Every argument is percent-encoded (RFC 3986, only ``A-Z a-z 0-9 - . _ ~`` kept), so
+        ``?``, ``#``, ``%`` and spaces stay inside their segment. Pass identifiers unencoded: an
+        already encoded value is encoded again. Enum members are sent by their value.
+
+        Args:
+            *params: the path segments, path constants and caller identifiers alike.
+
+        Returns:
+            str: the segments joined with ``/``.
+
+        Raises:
+            CheckoutArgumentException: a segment is not a string (or a string-valued Enum
+                member), is empty, ``.`` or ``..``, contains ``/``, ``\\`` or an ASCII control
+                character (U+0000 to U+001F, U+007F), or cannot be encoded as UTF-8.
+        """
+        return '/'.join(Client._encode_path_segment(param) for param in params)
+
+    @staticmethod
+    def _encode_path_segment(value):
+        # type() rather than isinstance(): a __class__ property cannot change it.
+        if issubclass(type(value), Enum):
+            value = value.value
+        if not issubclass(type(value), str):
+            raise CheckoutArgumentException(_INVALID_SEGMENT_MESSAGE)
+        # Exact str copy of the characters, so no overridden method of a str subclass takes part.
+        # A full slice copies on CPython and PyPy alike; str.__str__ keeps the subclass on PyPy.
+        value = str.__getitem__(value, slice(None))
+        if _FORBIDDEN_SEGMENT_CHARACTERS.search(value):
+            raise CheckoutArgumentException(_INVALID_SEGMENT_MESSAGE)
+        try:
+            encoded = quote(value, safe='')
+        except UnicodeEncodeError:
+            raise CheckoutArgumentException(_INVALID_SEGMENT_MESSAGE) from None
+        if encoded in ('', '.', '..'):
+            raise CheckoutArgumentException(_INVALID_SEGMENT_MESSAGE)
+        return encoded

checkout_sdk/events/events_client.py

@@ -19,10 +19,8 @@ def __init__(self, api_client: ApiClient, configuration: CheckoutConfiguration):
                          authorization_type=AuthorizationType.SECRET_KEY)
 
     def retrieve_all_event_types(self, version: str = None):
-        event_types_path = "event-types"
-        if version is not None:
-            event_types_path += "?version=" + version
-        return self._api_client.get(event_types_path, self._sdk_authorization())
+        params = None if version is None else {'version': version}
+        return self._api_client.get('event-types', self._sdk_authorization(), params)
 
     def retrieve_events(self, events_request: RetrieveEventsRequest):
         return self._api_client.get(self.__EVENTS_PATH, self._sdk_authorization(), events_request)

checkout_sdk/instruments/instruments_client.py

@@ -11,7 +11,8 @@
 
 class InstrumentsClient(Client):
     __INSTRUMENTS = 'instruments'
-    __BANK_VALIDATION_PATH = 'validation/bank-accounts'
+    __VALIDATION_PATH = 'validation'
+    __BANK_ACCOUNTS_PATH = 'bank-accounts'
 
     def __init__(self, api_client: ApiClient, configuration: CheckoutConfiguration):
         super().__init__(api_client=api_client,
@@ -40,5 +41,6 @@ def revoke(self, instrument_id: str):
 
     def get_bank_account_field_formatting(self, country: Country, currency: Currency,
                                           bank_account_field_query: BankAccountFieldQuery):
-        return self._api_client.get(self.build_path(self.__BANK_VALIDATION_PATH, country, currency),
+        return self._api_client.get(self.build_path(self.__VALIDATION_PATH, self.__BANK_ACCOUNTS_PATH,
+                                                    country, currency),
                                     self._sdk_authorization(AuthorizationType.OAUTH), bank_account_field_query)

checkout_sdk/payments/googlepay/googlepay_client.py

@@ -8,7 +8,8 @@
 
 
 class GooglePayClient(Client):
-    __GOOGLEPAY_ENROLLMENTS_PATH = 'googlepay/enrollments'
+    __GOOGLEPAY_PATH = 'googlepay'
+    __ENROLLMENTS_PATH = 'enrollments'
     __DOMAIN_PATH = 'domain'
     __DOMAINS_PATH = 'domains'
     __STATE_PATH = 'state'
@@ -19,20 +20,21 @@ def __init__(self, api_client: ApiClient, configuration: CheckoutConfiguration):
                          authorization_type=AuthorizationType.OAUTH)
 
     def create_enrollment(self, request: GooglePayEnrollmentRequest):
-        return self._api_client.post(self.__GOOGLEPAY_ENROLLMENTS_PATH, self._sdk_authorization(), request)
+        return self._api_client.post(self.build_path(self.__GOOGLEPAY_PATH, self.__ENROLLMENTS_PATH),
+                                     self._sdk_authorization(), request)
 
     def register_domain(self, entity_id: str, request: GooglePayRegisterDomainRequest):
         return self._api_client.post(
-            self.build_path(self.__GOOGLEPAY_ENROLLMENTS_PATH, entity_id, self.__DOMAIN_PATH),
+            self.build_path(self.__GOOGLEPAY_PATH, self.__ENROLLMENTS_PATH, entity_id, self.__DOMAIN_PATH),
             self._sdk_authorization(),
             request)
 
     def get_registered_domains(self, entity_id: str):
         return self._api_client.get(
-            self.build_path(self.__GOOGLEPAY_ENROLLMENTS_PATH, entity_id, self.__DOMAINS_PATH),
+            self.build_path(self.__GOOGLEPAY_PATH, self.__ENROLLMENTS_PATH, entity_id, self.__DOMAINS_PATH),
             self._sdk_authorization())
 
     def get_enrollment_state(self, entity_id: str):
         return self._api_client.get(
-            self.build_path(self.__GOOGLEPAY_ENROLLMENTS_PATH, entity_id, self.__STATE_PATH),
+            self.build_path(self.__GOOGLEPAY_PATH, self.__ENROLLMENTS_PATH, entity_id, self.__STATE_PATH),
             self._sdk_authorization())

checkout_sdk/payments/sessions/sessions_client.py

@@ -11,6 +11,8 @@
 
 class PaymentSessionsClient(Client):
     __PAYMENT_SESSIONS_PATH = 'payment-sessions'
+    __COMPLETE_PATH = 'complete'
+    __SUBMIT_PATH = 'submit'
 
     def __init__(self, api_client: ApiClient, configuration: CheckoutConfiguration):
         super().__init__(api_client=api_client,
@@ -23,11 +25,12 @@ def create_payment_sessions(self, payment_sessions_request: PaymentSessionsReque
 
     def create_payment_session_with_payment(
             self, payment_session_with_payment_request: PaymentSessionWithPaymentRequest):
-        return self._api_client.post(self.__PAYMENT_SESSIONS_PATH + '/complete', self._sdk_authorization(),
+        return self._api_client.post(self.build_path(self.__PAYMENT_SESSIONS_PATH, self.__COMPLETE_PATH),
+                                     self._sdk_authorization(),
                                      payment_session_with_payment_request)
 
     def submit_payment_session(self, session_id: str,
                                submit_payment_session_request: SubmitPaymentSessionRequest):
-        return self._api_client.post(self.__PAYMENT_SESSIONS_PATH + '/' + session_id + '/submit',
+        return self._api_client.post(self.build_path(self.__PAYMENT_SESSIONS_PATH, session_id, self.__SUBMIT_PATH),
                                      self._sdk_authorization(),
                                      submit_payment_session_request)