From 455b1f16bae38e6181cc06d0d98839148b5c2a8a Mon Sep 17 00:00:00 2001 From: bartes Date: Sun, 22 Mar 2020 19:55:43 +0100 Subject: [PATCH 1/8] added autopep8 config --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 pyproject.toml diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..256760f --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,2 @@ +[tool.autopep8] +max_line_length = 100 From 98dac3a36483c491938a88a3142e1497f44e9904 Mon Sep 17 00:00:00 2001 From: bartes Date: Sun, 22 Mar 2020 19:56:03 +0100 Subject: [PATCH 2/8] improved headers formatter --- castle/headers_formatter.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/castle/headers_formatter.py b/castle/headers_formatter.py index 5f297b0..a297083 100644 --- a/castle/headers_formatter.py +++ b/castle/headers_formatter.py @@ -4,8 +4,8 @@ class HeadersFormatter(object): @staticmethod def call(header): - return '-'.join([v.capitalize() for v in HeadersFormatter.split(header)]) + return HeadersFormatter.format(re.sub(r'^HTTP(?:_|-)', '', header, flags=re.IGNORECASE)) @staticmethod - def split(header): - return re.split(r'_|-', re.sub(r'^HTTP(?:_|-)', '', header, flags=re.IGNORECASE)) + def format(header): + return '-'.join([v.capitalize() for v in re.split(r'_|-', header)]) From 740425f26a96e45d620c28eda3777dfd4a641e79 Mon Sep 17 00:00:00 2001 From: bartes Date: Mon, 23 Mar 2020 02:42:16 +0100 Subject: [PATCH 3/8] added trusted proxies and formatted ip_headers, updated readme --- README.rst | 19 ++++++++++++++--- castle/configuration.py | 35 ++++++++++++++++++++++++------- castle/test/configuration_test.py | 16 +++++++++++--- 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/README.rst b/README.rst index 24cce37..d2e834a 100644 --- a/README.rst +++ b/README.rst @@ -44,9 +44,16 @@ import and configure the library with your Castle API secret. configuration.blacklisted = ['HTTP-X-header'] # Castle needs the original IP of the client, not the IP of your proxy or load balancer. - # If that IP is sent as a header you can configure the SDK to extract it automatically. - # Note that format, it should be prefixed with `HTTP`, capitalized and separated by underscores. - configuration.ip_headers = ["HTTP_X_FORWARDED_FOR"] + # we try to fetch proper ip based on X-Forwarded-For, X-Client-Id or Remote-Addr headers in that order + # but sometimes proper ip may be stored in different header or order could be different. + # SDK can extract ip automatically for you, but you must configure which ip_headers you would like to use + configuration.ip_headers = [] + # Additionally to make X-Forwarded-For or X-Client-Id work better discovering client ip address, + # and not the address of a reverse proxy server, you can define trusted proxies + # which will help to fetch proper ip from those headers + configuration.trusted_proxies = [] + # *Note: proxies list can be provided as an array of regular expressions + # *Note: default always marked as trusty list is here: Castle::Configuration::TRUSTED_PROXIES Tracking -------- @@ -109,6 +116,12 @@ and use it later in a way client = Client(context) client.track(options) +## Events + +List of Recognized Events can be found [here](https://github.com/castle/castle-python/tree/master/castle/events.py) or in the [docs](https://docs.castle.io/api_reference/#list-of-recognized-events) + + + Impersonation mode ------------------ diff --git a/castle/configuration.py b/castle/configuration.py index 56188f5..9e9d181 100644 --- a/castle/configuration.py +++ b/castle/configuration.py @@ -1,7 +1,7 @@ from castle.exceptions import ConfigurationError from castle.headers_formatter import HeadersFormatter -WHITELISTED = [ +DEFAULT_WHITELIST = [ "Accept", "Accept-Charset", "Accept-Datetime", @@ -25,19 +25,30 @@ # 500 milliseconds REQUEST_TIMEOUT = 500 FAILOVER_STRATEGIES = ['allow', 'deny', 'challenge', 'throw'] - +HOST = 'api.castle.io' +PORT = 443 +URL_PREFIX = '/v1' +FAILOVER_STRATEGY = 'allow' +TRUSTED_PROXIES = [r""" + \A127\.0\.0\.1\Z| + \A(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\.| + \A::1\Z|\Afd[0-9a-f]{2}:.+| + \Alocalhost\Z| + \Aunix\Z| + \Aunix:"""] class Configuration(object): def __init__(self): self.api_secret = None - self.host = 'api.castle.io' - self.port = 443 - self.url_prefix = '/v1' + self.host = HOST + self.port = PORT + self.url_prefix = URL_PREFIX self.whitelisted = [] self.blacklisted = [] self.request_timeout = REQUEST_TIMEOUT - self.failover_strategy = 'allow' + self.failover_strategy = FAILOVER_STRATEGY self.ip_headers = [] + self.trusted_proxies = [] @property def api_secret(self): @@ -119,10 +130,20 @@ def ip_headers(self): @ip_headers.setter def ip_headers(self, value): if isinstance(value, list): - self.__ip_headers = value + self.__ip_headers = [HeadersFormatter.call(v) for v in value] else: raise ConfigurationError + @property + def trusted_proxies(self): + return self.__trusted_proxies + + @trusted_proxies.setter + def trusted_proxies(self, value): + if isinstance(value, list): + self.__trusted_proxies = value + else: + raise ConfigurationError # pylint: disable=invalid-name configuration = Configuration() diff --git a/castle/test/configuration_test.py b/castle/test/configuration_test.py index 761f5b3..938e0d5 100644 --- a/castle/test/configuration_test.py +++ b/castle/test/configuration_test.py @@ -16,6 +16,7 @@ def test_default_values(self): self.assertEqual(config.request_timeout, 500) self.assertEqual(config.failover_strategy, 'allow') self.assertEqual(config.ip_headers, []) + self.assertEqual(config.trusted_proxies, []) def test_api_secret_setter(self): config = Configuration() @@ -84,11 +85,20 @@ def test_failover_strategy_setter_invalid(self): def test_ip_headers_setter_valid(self): config = Configuration() - ip_headers = ['HTTP_X_FORWARDED_FOR'] - config.ip_headers = ip_headers - self.assertEqual(config.ip_headers, ip_headers) + config.ip_headers = ['HTTP_X_FORWARDED_FOR'] + self.assertEqual(config.ip_headers, ['X-Forwarded-For']) def test_ip_headers_setter_invalid(self): config = Configuration() with self.assertRaises(ConfigurationError): config.ip_headers = 'invalid' + + def test_trusted_proxies_setter_valid(self): + config = Configuration() + config.trusted_proxies = ['2.2.2.2'] + self.assertEqual(config.trusted_proxies, ['2.2.2.2']) + + def test_trusted_proxies_setter_invalid(self): + config = Configuration() + with self.assertRaises(ConfigurationError): + config.trusted_proxies = 'invalid' From fa8b36145385448c06b0a0eeea16d6d14c4f6dba Mon Sep 17 00:00:00 2001 From: bartes Date: Mon, 23 Mar 2020 02:46:10 +0100 Subject: [PATCH 4/8] removed not used imports --- castle/client.py | 2 +- castle/commands/identify.py | 1 - castle/context/sanitizer.py | 8 ++++---- castle/request.py | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/castle/client.py b/castle/client.py index 75db2a4..deb2612 100644 --- a/castle/client.py +++ b/castle/client.py @@ -1,3 +1,4 @@ +import warnings from castle.configuration import configuration from castle.api import Api from castle.context.default import ContextDefault @@ -9,7 +10,6 @@ from castle.exceptions import InternalServerError, RequestError, ImpersonationFailed from castle.failover_response import FailoverResponse from castle.utils import timestamp as generate_timestamp -import warnings class Client(object): diff --git a/castle/commands/identify.py b/castle/commands/identify.py index 8d72cae..debe9aa 100644 --- a/castle/commands/identify.py +++ b/castle/commands/identify.py @@ -2,7 +2,6 @@ from castle.utils import timestamp from castle.context.merger import ContextMerger from castle.context.sanitizer import ContextSanitizer -from castle.validators.present import ValidatorsPresent from castle.validators.not_supported import ValidatorsNotSupported diff --git a/castle/context/sanitizer.py b/castle/context/sanitizer.py index 6efa83e..49ec5aa 100644 --- a/castle/context/sanitizer.py +++ b/castle/context/sanitizer.py @@ -7,13 +7,13 @@ def call(cls, context): return sanitized_context return dict() - @classmethod - def _sanitize_active_mode(cls, context): + @staticmethod + def _sanitize_active_mode(context): if context is None: return None - elif 'active' not in context: + if 'active' not in context: return context - elif isinstance(context.get('active'), bool): + if isinstance(context.get('active'), bool): return context context_copy = context.copy() diff --git a/castle/request.py b/castle/request.py index 2cb3274..5b0ab0e 100644 --- a/castle/request.py +++ b/castle/request.py @@ -38,4 +38,4 @@ def build_base_url(): @staticmethod def verify(): - return True if configuration.port == 443 else False + return configuration.port == 443 From 9fe3bd3318b66093de37d3f74e06d5975d2aa12e Mon Sep 17 00:00:00 2001 From: bartes Date: Mon, 23 Mar 2020 02:46:29 +0100 Subject: [PATCH 5/8] added missing events --- castle/events.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/castle/events.py b/castle/events.py index c884387..d9310af 100644 --- a/castle/events.py +++ b/castle/events.py @@ -35,3 +35,8 @@ CHALLENGE_SUCCEEDED = '$challenge.succeeded' # Record when additional verification failed. CHALLENGE_FAILED = '$challenge.failed' +# Record when a user attempts an in-app transaction, such as a purchase or withdrawal. +TRANSACTION_ATTEMPTED = '$transaction.attempted' +# Record when a user session is extended, or use any time you want +# to re-authenticate a user mid-session. +SESSION_EXTENDED = '$session.extended' From 9eabd29fcb6c67d1f52f5e36978fcacf53c38538 Mon Sep 17 00:00:00 2001 From: bartes Date: Mon, 23 Mar 2020 02:48:34 +0100 Subject: [PATCH 6/8] refactored headers management, added ip_headers defaults, extract ip from headers --- castle/context/default.py | 55 +++++++++------ castle/extractors/client_id.py | 6 +- castle/extractors/headers.py | 43 ++++++------ castle/extractors/ip.py | 59 ++++++++++++---- castle/headers_filter.py | 25 +++++++ castle/headers_formatter.py | 2 +- castle/test/context/default_test.py | 27 ++----- castle/test/extractors/client_id_test.py | 2 +- castle/test/extractors/headers_test.py | 89 +++++++++++++++++++----- castle/test/extractors/ip_test.py | 82 +++++++++------------- castle/test/headers_filter_test.py | 31 +++++++++ 11 files changed, 270 insertions(+), 151 deletions(-) create mode 100644 castle/headers_filter.py create mode 100644 castle/test/headers_filter_test.py diff --git a/castle/context/default.py b/castle/context/default.py index 480fd97..17d9228 100644 --- a/castle/context/default.py +++ b/castle/context/default.py @@ -1,4 +1,5 @@ from castle.version import VERSION +from castle.headers_filter import HeadersFilter from castle.extractors.client_id import ExtractorsClientId from castle.extractors.headers import ExtractorsHeaders from castle.extractors.ip import ExtractorsIp @@ -8,39 +9,47 @@ class ContextDefault(object): def __init__(self, request, cookies): - used_cookies = self._fetch_cookies(request, cookies) - self.client_id = ExtractorsClientId( - request.environ, used_cookies).call() - self.headers = ExtractorsHeaders(request.environ).call() - self.request_ip = ExtractorsIp(request).call() - - def _defaults(self): - return { - 'client_id': self.client_id, + self.cookies = self._fetch_cookies(request, cookies) + self.pre_headers = HeadersFilter(request).call() + + def call(self): + context = dict({ + 'client_id': self._client_id(), 'active': True, 'origin': 'web', - 'headers': self.headers, - 'ip': self.request_ip, - 'library': {'name': 'castle-python', 'version': __version__} - } + 'headers': self._headers(), + 'ip': self._ip(), + 'library': { + 'name': 'castle-python', + 'version': __version__ + } + }) + context.update(self._optional_defaults()) - def _defaults_extra(self): - context = dict() - if 'Accept-Language' in self.headers: - context['locale'] = self.headers['Accept-Language'] - if 'User-Agent' in self.headers: - context['user_agent'] = self.headers['User-Agent'] return context - def call(self): - context = dict(self._defaults()) - context.update(self._defaults_extra()) + def _ip(self): + return ExtractorsIp(self.pre_headers).call() + + def _client_id(self): + return ExtractorsClientId(self.pre_headers, self.cookies).call() + + def _headers(self): + return ExtractorsHeaders(self.pre_headers).call() + + def _optional_defaults(self): + context = dict() + if 'Accept-Language' in self.pre_headers: + context['locale'] = self.pre_headers.get('Accept-Language') + if 'User-Agent' in self.pre_headers: + context['user_agent'] = self.pre_headers.get('User-Agent') return context + @staticmethod def _fetch_cookies(request, cookies): if cookies: return cookies - elif hasattr(request, 'COOKIES') and request.COOKIES: + if hasattr(request, 'COOKIES') and request.COOKIES: return request.COOKIES return None diff --git a/castle/extractors/client_id.py b/castle/extractors/client_id.py index 11dcb76..4869b49 100644 --- a/castle/extractors/client_id.py +++ b/castle/extractors/client_id.py @@ -1,7 +1,7 @@ class ExtractorsClientId(object): - def __init__(self, environ, cookies=None): - self.environ = environ + def __init__(self, headers, cookies=None): + self.headers = headers self.cookies = cookies or dict() def call(self): - return self.environ.get('HTTP_X_CASTLE_CLIENT_ID', self.cookies.get('__cid', '')) + return self.headers.get('X-Castle-Client-Id', self.cookies.get('__cid', '')) diff --git a/castle/extractors/headers.py b/castle/extractors/headers.py index 20fec29..d42b712 100644 --- a/castle/extractors/headers.py +++ b/castle/extractors/headers.py @@ -1,27 +1,30 @@ -from castle.headers_formatter import HeadersFormatter from castle.configuration import configuration -DEFAULT_BLACKLIST = ['Cookie', 'Authorization'] -DEFAULT_WHITELIST = ['User-Agent'] +ALWAYS_BLACKLISTED = ['Cookie', 'Authorization'] +ALWAYS_WHITELISTED = ['User-Agent'] class ExtractorsHeaders(object): - def __init__(self, environ): - self.environ = environ - self.formatter = HeadersFormatter + def __init__(self, headers): + self.headers = headers + self.no_whitelist = len(configuration.whitelisted) == 0 def call(self): - headers = dict() - has_whitelist = len(configuration.whitelisted) > 0 - - for key, value in self.environ.items(): - name = self.formatter.call(key) - if has_whitelist and name not in configuration.whitelisted and name not in DEFAULT_WHITELIST: - headers[name] = True - continue - if name in configuration.blacklisted or name in DEFAULT_BLACKLIST: - headers[name] = True - continue - headers[name] = value - - return headers + result = dict() + + for name, value in self.headers.items(): + result[name] = self._header_value(name, value) + + return result + + def _header_value(self, name, value): + if name in ALWAYS_BLACKLISTED: + return True + if name in ALWAYS_WHITELISTED: + return value + if name in configuration.blacklisted: + return True + if self.no_whitelist or (name in configuration.whitelisted): + return value + + return True diff --git a/castle/extractors/ip.py b/castle/extractors/ip.py index a762fd3..4854eba 100644 --- a/castle/extractors/ip.py +++ b/castle/extractors/ip.py @@ -1,23 +1,54 @@ -from castle.configuration import configuration +import re +from castle.configuration import configuration, TRUSTED_PROXIES +# ordered list of ip headers for ip extraction +DEFAULT = ['X-Forwarded-For', 'Client-Ip', 'Remote-Addr'] +# default header fallback when ip is not found +FALLBACK = 'Remote-Addr' + class ExtractorsIp(object): - def __init__(self, request): - self.request = request + def __init__(self, headers): + self.headers = headers + self.ip_headers = configuration.ip_headers + DEFAULT + self.proxies = configuration.trusted_proxies + TRUSTED_PROXIES def call(self): - ip_address = self.get_ip_from_headers() - if ip_address: - return ip_address + for ip_header in self.ip_headers: + ip_value = self._calculate_ip(ip_header) + if ip_value: + return ip_value - if hasattr(self.request, 'ip'): - return self.request.ip + return self.headers.get(FALLBACK, None) - return self.request.environ.get('REMOTE_ADDR') + def _calculate_ip(self, header): + ips = self._ips_from(header) + filtered_ips = self._remove_proxies(ips) - def get_ip_from_headers(self): - for header in configuration.ip_headers: - value = self.request.environ.get(header) - if value: - return value + if len(filtered_ips) > 0: + return filtered_ips[0] return None + + def _remove_proxies(self, ips): + result = [] + + for ip_address in ips: + if not self._is_proxy(ip_address): + result.append(ip_address) + return result + + def _is_proxy(self, ip_address): + for proxy_re in self.proxies: + if re.match(proxy_re, ip_address, flags=re.I|re.X): + return True + + return False + + + def _ips_from(self, header): + value = self.headers.get(header) + + if not value: + return [] + + return re.split(r'[,\s]+', value.strip())[::-1] diff --git a/castle/headers_filter.py b/castle/headers_filter.py new file mode 100644 index 0000000..a01ad17 --- /dev/null +++ b/castle/headers_filter.py @@ -0,0 +1,25 @@ +import re +from castle.headers_formatter import HeadersFormatter + +VALUABLE_HEADERS = r"""^ + HTTP(?:_|-).*| + CONTENT(?:_|-)LENGTH| + REMOTE(?:_|-)ADDR +$""" + +class HeadersFilter(object): + def __init__(self, request): + self.environ = request.environ + self.formatter = HeadersFormatter + + def call(self): + result = dict() + + for header_name, value in self.environ.items(): + if not re.match(VALUABLE_HEADERS, header_name, flags=re.X|re.I): + continue + + formatted_name = HeadersFormatter.call(header_name) + result[formatted_name] = value + + return result diff --git a/castle/headers_formatter.py b/castle/headers_formatter.py index a297083..98535c3 100644 --- a/castle/headers_formatter.py +++ b/castle/headers_formatter.py @@ -4,7 +4,7 @@ class HeadersFormatter(object): @staticmethod def call(header): - return HeadersFormatter.format(re.sub(r'^HTTP(?:_|-)', '', header, flags=re.IGNORECASE)) + return HeadersFormatter.format(re.sub(r'^HTTP(?:_|-)', '', header, flags=re.I)) @staticmethod def format(header): diff --git a/castle/test/context/default_test.py b/castle/test/context/default_test.py index 1c45594..c7361fc 100644 --- a/castle/test/context/default_test.py +++ b/castle/test/context/default_test.py @@ -13,25 +13,16 @@ def cookies(): def request_ip(): - return '127.0.0.1' + return '5.5.5.5' def environ(): return { 'HTTP_X_FORWARDED_FOR': request_ip(), - 'HTTP_COOKIE': "__cid={client_id()};other=efgh" - } - - -def environ_with_extras(): - extra = { + 'HTTP_COOKIE': "__cid={client_id()};other=efgh", 'HTTP-Accept-Language': 'en', 'HTTP-User-Agent': 'test' } - context = dict(environ()) - context.update(extra) - return context - def request(env): req = mock.Mock() @@ -41,19 +32,10 @@ def request(env): class ContextDefaultTestCase(unittest.TestCase): - def test_default_context(self): - context = ContextDefault(request(environ()), cookies()).call() - self.assertEqual(context['client_id'], client_id()) - self.assertEqual(context['active'], True) - self.assertEqual(context['origin'], 'web') - self.assertEqual(context['headers'], {'X-Forwarded-For': request_ip(), 'Cookie': True}) - self.assertEqual(context['ip'], request_ip()) - self.assertDictEqual(context['library'], { - 'name': 'castle-python', 'version': __version__}) - def test_default_context_with_extras(self): + def test_default_context(self): context = ContextDefault( - request(environ_with_extras()), cookies()).call() + request(environ()), cookies()).call() self.assertEqual(context['client_id'], client_id()) self.assertEqual(context['active'], True) self.assertEqual(context['origin'], 'web') @@ -71,3 +53,4 @@ def test_default_context_with_extras(self): context['library'], {'name': 'castle-python', 'version': __version__} ) + self.assertEqual(context['user_agent'], 'test') diff --git a/castle/test/extractors/client_id_test.py b/castle/test/extractors/client_id_test.py index a3cb18f..e1100fa 100644 --- a/castle/test/extractors/client_id_test.py +++ b/castle/test/extractors/client_id_test.py @@ -16,7 +16,7 @@ def cookies(): def environ(): - return {'HTTP_X_CASTLE_CLIENT_ID': client_id_environ()} + return {'X-Castle-Client-Id': client_id_environ()} class ExtractorsClientIdTestCase(unittest.TestCase): diff --git a/castle/test/extractors/headers_test.py b/castle/test/extractors/headers_test.py index 8847a1a..f8f6e3c 100644 --- a/castle/test/extractors/headers_test.py +++ b/castle/test/extractors/headers_test.py @@ -1,31 +1,82 @@ from castle.test import unittest -from castle.configuration import configuration, WHITELISTED +from castle.configuration import configuration from castle.extractors.headers import ExtractorsHeaders -def client_id(): - return 'abcd' +def formatted_headers(): + return dict({ + 'Content-Length': '0', + 'Authorization': 'Basic 123456', + 'Cookie': '__cid=abcd;other=efgh', + 'Ok': 'OK', + 'Accept': 'application/json', + 'X-Forwarded-For': '1.2.3.4', + 'User-Agent': 'Mozilla 1234' + }) -def environ(): - return { - 'HTTP_USER_AGENT': 'requests', - 'HTTP_OK': 'OK', - 'TEST': '1', - 'HTTP_COOKIE': "__cid={client_id};other=efgh".format(client_id=client_id) - } +class ExtractorsHeadersTestCase(unittest.TestCase): + def tearDown(self): + configuration.whitelisted = [] + configuration.blacklisted = [] -class ExtractorsHeadersTestCase(unittest.TestCase): def test_extract_headers(self): - configuration.whitelisted = [] - self.assertEqual(ExtractorsHeaders(environ()).call(), - {'User-Agent': 'requests', 'Ok': 'OK', 'Test': '1', 'Cookie': True}) + self.assertEqual(ExtractorsHeaders(formatted_headers()).call(), + {'Accept': 'application/json', + 'Authorization': True, + 'Cookie': True, + 'Content-Length': '0', + 'Ok': 'OK', + 'User-Agent': 'Mozilla 1234', + 'X-Forwarded-For': '1.2.3.4' + }) - def test_add_whitelisted_headers(self): - configuration.whitelisted = WHITELISTED + ['TEST'] + def test_whitelisted_headers(self): + configuration.whitelisted = ['Accept', 'OK'] self.assertEqual( - ExtractorsHeaders(environ()).call(), - {'User-Agent': 'requests', 'Test': '1', 'Cookie': True, 'Ok': True} + ExtractorsHeaders(formatted_headers()).call(), + {'Accept': 'application/json', + 'Authorization': True, + 'Cookie': True, + 'Content-Length': True, + 'Ok': 'OK', + 'User-Agent': 'Mozilla 1234', + 'X-Forwarded-For': True + } + ) +# + def test_restricted_blacklisted_headers(self): + configuration.blacklisted = ['User-Agent'] + self.assertEqual( + ExtractorsHeaders(formatted_headers()).call(), + {'Accept': 'application/json', + 'Authorization': True, + 'Cookie': True, + 'Content-Length': '0', + 'Ok': 'OK', + 'User-Agent': 'Mozilla 1234', + 'X-Forwarded-For': '1.2.3.4' + } + ) + + def test_blacklisted_headers(self): + configuration.blacklisted = ['Accept'] + self.assertEqual( + ExtractorsHeaders(formatted_headers()).call(), + {'Accept': True, + 'Authorization': True, + 'Cookie': True, + 'Content-Length': '0', + 'Ok': 'OK', + 'User-Agent': 'Mozilla 1234', + 'X-Forwarded-For': '1.2.3.4' + } + ) +# + def test_blacklisted_and_whitelisted_headers(self): + configuration.blacklisted = ['Accept'] + configuration.whitelisted = ['Accept'] + self.assertEqual( + ExtractorsHeaders(formatted_headers()).call()['Accept'], True ) - configuration.whitelisted = [] diff --git a/castle/test/extractors/ip_test.py b/castle/test/extractors/ip_test.py index ce56da2..9f78956 100644 --- a/castle/test/extractors/ip_test.py +++ b/castle/test/extractors/ip_test.py @@ -3,64 +3,50 @@ from castle.configuration import configuration -def request_ip(): - return '127.0.0.1' - - -def request_ip_next(): - return '127.0.0.2' - - -def request(): - req = mock.Mock(spec=['ip']) - req.ip = request_ip() - return req - - -def request_with_ip_remote_addr(): - req = mock.Mock(spec=['environ']) - req.environ = {'REMOTE_ADDR': request_ip()} - return req - - -def request_with_ip_x_forwarded_for(): - req = mock.Mock(spec=['environ']) - req.environ = {'HTTP_X_FORWARDED_FOR': request_ip()} - return req - - -def request_with_ip_cf_connecting_ip(): - req = mock.Mock(spec=['environ']) - req.environ = {'HTTP_CF_CONNECTING_IP': request_ip_next()} - return req - - class ExtractorsIpTestCase(unittest.TestCase): - @classmethod - def tearDownClass(cls): + def tearDown(self): configuration.ip_headers = [] + configuration.trusted_proxies = [] def test_extract_ip(self): - self.assertEqual(ExtractorsIp(request()).call(), request_ip()) + headers = { 'X-Forwarded-For': '1.2.3.5' } + self.assertEqual(ExtractorsIp(headers).call(), '1.2.3.5') - def test_extract_ip_from_wsgi_request_remote_addr(self): + def test_extract_ip_when_second_header(self): + headers = { 'Cf-Connecting-Ip': '1.2.3.4', 'X-Forwarded-For': '1.1.1.1, 1.2.2.2, 1.2.3.5' } + configuration.ip_headers = ["HTTP_CF_CONNECTING_IP"] self.assertEqual( - ExtractorsIp(request_with_ip_remote_addr()).call(), - request_ip() + ExtractorsIp(headers).call(), + '1.2.3.4' + ) + def test_extract_ip_when_second_header_with_different_setting(self): + headers = { 'Cf-Connecting-Ip': '1.2.3.4', 'X-Forwarded-For': '1.1.1.1, 1.2.2.2, 1.2.3.5' } + configuration.ip_headers = ["CF-CONNECTING-IP"] + self.assertEqual( + ExtractorsIp(headers).call(), + '1.2.3.4' ) - def test_extract_ip_from_wsgi_request_configured_ip_header_first(self): - configuration.ip_headers = ["HTTP_CF_CONNECTING_IP"] + def test_extract_ip_when_all_trusted_proxies(self): + xf_header = '127.0.0.1,10.0.0.1,172.31.0.1,192.168.0.1,::1,fd00::,localhost,unix,unix:/tmp/sock' + headers = { 'Remote-Addr': '127.0.0.1', 'X-Forwarded-For': xf_header } self.assertEqual( - ExtractorsIp(request_with_ip_cf_connecting_ip()).call(), - request_ip_next() + ExtractorsIp(headers).call(), + '127.0.0.1' ) - configuration.ip_headers = [] - def test_extract_ip_from_wsgi_request_configured_ip_header_second(self): - configuration.ip_headers = ["HTTP_CF_CONNECTING_IP", "HTTP_X_FORWARDED_FOR"] + + def test_extract_ip_for_spoof_ip_attempt(self): + headers = { 'Client-Ip': '6.6.6.6', 'X-Forwarded-For': '6.6.6.6, 2.2.2.3, 192.168.0.7' } self.assertEqual( - ExtractorsIp(request_with_ip_x_forwarded_for()).call(), - request_ip() + ExtractorsIp(headers).call(), + '2.2.2.3' + ) +# + def test_extract_ip_for_spoof_ip_attempt_when_all_trusted_proxies(self): + headers = { 'Client-Ip': '6.6.6.6', 'X-Forwarded-For': '6.6.6.6, 2.2.2.3, 192.168.0.7' } + configuration.trusted_proxies = ['^2.2.2.\d$'] + self.assertEqual( + ExtractorsIp(headers).call(), + '6.6.6.6' ) - configuration.ip_headers = [] diff --git a/castle/test/headers_filter_test.py b/castle/test/headers_filter_test.py new file mode 100644 index 0000000..4f9e444 --- /dev/null +++ b/castle/test/headers_filter_test.py @@ -0,0 +1,31 @@ +from castle.test import unittest +from castle.headers_filter import HeadersFilter + + +def headers(): + return { + 'Action-Dispatch.request.content-Type': 'application/json', + 'HTTP_AUTHORIZATION': 'Basic 123456', + 'HTTP_COOKIE': "__cid=abcd;other=efgh", + 'HTTP_OK': 'OK', + 'HTTP_ACCEPT': 'application/json', + 'HTTP_X_FORWARDED_FOR': '1.2.3.4', + 'HTTP_USER_AGENT': 'Mozilla 1234', + 'TEST': '1', + 'REMOTE_ADDR': '1.2.3.4' + } + + +class ExtractorsHeadersTestCase(unittest.TestCase): + def test_filter_headers(self): + self.assertEqual(HeadersFilter(headers()).call(), + { + 'Accept': 'application/json', + 'Authorization': 'Basic 123456', + 'Cookie': "__cid=abcd;other=efgh", + 'Content-Length': '0', + 'Ok': 'OK', + 'User-Agent': 'Mozilla 1234', + 'Remote-Addr': '1.2.3.4', + 'X-Forwarded-For': '1.2.3.4' + }) From 94881f52b5f2bddc8a05fe4071d9ca64ebe80578 Mon Sep 17 00:00:00 2001 From: bartes Date: Mon, 23 Mar 2020 02:50:22 +0100 Subject: [PATCH 7/8] linter fixes and improved formatting --- castle/client.py | 1 + castle/configuration.py | 2 ++ castle/context/default.py | 1 - castle/exceptions.py | 10 +++++++ castle/extractors/ip.py | 4 +-- castle/headers_filter.py | 3 +- castle/test/commands/impersonate_test.py | 1 + castle/test/configuration_test.py | 1 - castle/test/context/default_test.py | 1 + castle/test/extractors/headers_test.py | 2 ++ castle/test/extractors/ip_test.py | 23 ++++++++------- castle/test/headers_filter_test.py | 36 ++++++++++++------------ castle/test/request_test.py | 4 +-- pylintrc | 5 ++-- 14 files changed, 56 insertions(+), 38 deletions(-) diff --git a/castle/client.py b/castle/client.py index deb2612..c60adeb 100644 --- a/castle/client.py +++ b/castle/client.py @@ -11,6 +11,7 @@ from castle.failover_response import FailoverResponse from castle.utils import timestamp as generate_timestamp + class Client(object): @classmethod diff --git a/castle/configuration.py b/castle/configuration.py index 9e9d181..0b55c3d 100644 --- a/castle/configuration.py +++ b/castle/configuration.py @@ -37,6 +37,7 @@ \Aunix\Z| \Aunix:"""] + class Configuration(object): def __init__(self): self.api_secret = None @@ -145,5 +146,6 @@ def trusted_proxies(self, value): else: raise ConfigurationError + # pylint: disable=invalid-name configuration = Configuration() diff --git a/castle/context/default.py b/castle/context/default.py index 17d9228..167aa96 100644 --- a/castle/context/default.py +++ b/castle/context/default.py @@ -45,7 +45,6 @@ def _optional_defaults(self): context['user_agent'] = self.pre_headers.get('User-Agent') return context - @staticmethod def _fetch_cookies(request, cookies): if cookies: diff --git a/castle/exceptions.py b/castle/exceptions.py index a846065..13dd868 100644 --- a/castle/exceptions.py +++ b/castle/exceptions.py @@ -6,12 +6,15 @@ class CastleError(Exception): class RequestError(CastleError): pass + class SecurityError(CastleError): pass + class ConfigurationError(CastleError): pass + class ApiError(CastleError): pass @@ -19,23 +22,30 @@ class ApiError(CastleError): class InvalidParametersError(ApiError): pass + class BadRequestError(ApiError): pass + class UnauthorizedError(ApiError): pass + class UserUnauthorizedError(ApiError): pass + class ForbiddenError(ApiError): pass + class NotFoundError(ApiError): pass + class InternalServerError(ApiError): pass + class ImpersonationFailed(ApiError): pass diff --git a/castle/extractors/ip.py b/castle/extractors/ip.py index 4854eba..e84e7cc 100644 --- a/castle/extractors/ip.py +++ b/castle/extractors/ip.py @@ -7,6 +7,7 @@ # default header fallback when ip is not found FALLBACK = 'Remote-Addr' + class ExtractorsIp(object): def __init__(self, headers): self.headers = headers @@ -39,12 +40,11 @@ def _remove_proxies(self, ips): def _is_proxy(self, ip_address): for proxy_re in self.proxies: - if re.match(proxy_re, ip_address, flags=re.I|re.X): + if re.match(proxy_re, ip_address, flags=re.I | re.X): return True return False - def _ips_from(self, header): value = self.headers.get(header) diff --git a/castle/headers_filter.py b/castle/headers_filter.py index a01ad17..55ec912 100644 --- a/castle/headers_filter.py +++ b/castle/headers_filter.py @@ -7,6 +7,7 @@ REMOTE(?:_|-)ADDR $""" + class HeadersFilter(object): def __init__(self, request): self.environ = request.environ @@ -16,7 +17,7 @@ def call(self): result = dict() for header_name, value in self.environ.items(): - if not re.match(VALUABLE_HEADERS, header_name, flags=re.X|re.I): + if not re.match(VALUABLE_HEADERS, header_name, flags=re.X | re.I): continue formatted_name = HeadersFormatter.call(header_name) diff --git a/castle/test/commands/impersonate_test.py b/castle/test/commands/impersonate_test.py index e3251bf..d112549 100644 --- a/castle/test/commands/impersonate_test.py +++ b/castle/test/commands/impersonate_test.py @@ -26,6 +26,7 @@ def default_command_with_data(**data): data=dict(sent_at=mock.sentinel.timestamp, **data) ) + def default_reset_command_with_data(**data): """What we expect the impersonate command to look like.""" return Command( diff --git a/castle/test/configuration_test.py b/castle/test/configuration_test.py index 938e0d5..a2aedf1 100644 --- a/castle/test/configuration_test.py +++ b/castle/test/configuration_test.py @@ -1,7 +1,6 @@ from castle.test import unittest from castle.exceptions import ConfigurationError from castle.configuration import Configuration -from castle.headers_formatter import HeadersFormatter class ConfigurationTestCase(unittest.TestCase): diff --git a/castle/test/context/default_test.py b/castle/test/context/default_test.py index c7361fc..d16f2f2 100644 --- a/castle/test/context/default_test.py +++ b/castle/test/context/default_test.py @@ -24,6 +24,7 @@ def environ(): 'HTTP-User-Agent': 'test' } + def request(env): req = mock.Mock() req.ip = request_ip() diff --git a/castle/test/extractors/headers_test.py b/castle/test/extractors/headers_test.py index f8f6e3c..14ab93f 100644 --- a/castle/test/extractors/headers_test.py +++ b/castle/test/extractors/headers_test.py @@ -46,6 +46,7 @@ def test_whitelisted_headers(self): } ) # + def test_restricted_blacklisted_headers(self): configuration.blacklisted = ['User-Agent'] self.assertEqual( @@ -74,6 +75,7 @@ def test_blacklisted_headers(self): } ) # + def test_blacklisted_and_whitelisted_headers(self): configuration.blacklisted = ['Accept'] configuration.whitelisted = ['Accept'] diff --git a/castle/test/extractors/ip_test.py b/castle/test/extractors/ip_test.py index 9f78956..eb88d99 100644 --- a/castle/test/extractors/ip_test.py +++ b/castle/test/extractors/ip_test.py @@ -1,4 +1,4 @@ -from castle.test import unittest, mock +from castle.test import unittest from castle.extractors.ip import ExtractorsIp from castle.configuration import configuration @@ -9,18 +9,19 @@ def tearDown(self): configuration.trusted_proxies = [] def test_extract_ip(self): - headers = { 'X-Forwarded-For': '1.2.3.5' } + headers = {'X-Forwarded-For': '1.2.3.5'} self.assertEqual(ExtractorsIp(headers).call(), '1.2.3.5') def test_extract_ip_when_second_header(self): - headers = { 'Cf-Connecting-Ip': '1.2.3.4', 'X-Forwarded-For': '1.1.1.1, 1.2.2.2, 1.2.3.5' } + headers = {'Cf-Connecting-Ip': '1.2.3.4', 'X-Forwarded-For': '1.1.1.1, 1.2.2.2, 1.2.3.5'} configuration.ip_headers = ["HTTP_CF_CONNECTING_IP"] self.assertEqual( ExtractorsIp(headers).call(), '1.2.3.4' ) + def test_extract_ip_when_second_header_with_different_setting(self): - headers = { 'Cf-Connecting-Ip': '1.2.3.4', 'X-Forwarded-For': '1.1.1.1, 1.2.2.2, 1.2.3.5' } + headers = {'Cf-Connecting-Ip': '1.2.3.4', 'X-Forwarded-For': '1.1.1.1, 1.2.2.2, 1.2.3.5'} configuration.ip_headers = ["CF-CONNECTING-IP"] self.assertEqual( ExtractorsIp(headers).call(), @@ -28,24 +29,26 @@ def test_extract_ip_when_second_header_with_different_setting(self): ) def test_extract_ip_when_all_trusted_proxies(self): - xf_header = '127.0.0.1,10.0.0.1,172.31.0.1,192.168.0.1,::1,fd00::,localhost,unix,unix:/tmp/sock' - headers = { 'Remote-Addr': '127.0.0.1', 'X-Forwarded-For': xf_header } + xf_header = """ + 127.0.0.1,10.0.0.1,172.31.0.1,192.168.0.1,::1,fd00::,localhost,unix,unix:/tmp/sock + """ + headers = {'Remote-Addr': '127.0.0.1', 'X-Forwarded-For': xf_header} self.assertEqual( ExtractorsIp(headers).call(), '127.0.0.1' ) - def test_extract_ip_for_spoof_ip_attempt(self): - headers = { 'Client-Ip': '6.6.6.6', 'X-Forwarded-For': '6.6.6.6, 2.2.2.3, 192.168.0.7' } + headers = {'Client-Ip': '6.6.6.6', 'X-Forwarded-For': '6.6.6.6, 2.2.2.3, 192.168.0.7'} self.assertEqual( ExtractorsIp(headers).call(), '2.2.2.3' ) # + def test_extract_ip_for_spoof_ip_attempt_when_all_trusted_proxies(self): - headers = { 'Client-Ip': '6.6.6.6', 'X-Forwarded-For': '6.6.6.6, 2.2.2.3, 192.168.0.7' } - configuration.trusted_proxies = ['^2.2.2.\d$'] + headers = {'Client-Ip': '6.6.6.6', 'X-Forwarded-For': '6.6.6.6, 2.2.2.3, 192.168.0.7'} + configuration.trusted_proxies = [r'^2.2.2.\d$'] self.assertEqual( ExtractorsIp(headers).call(), '6.6.6.6' diff --git a/castle/test/headers_filter_test.py b/castle/test/headers_filter_test.py index 4f9e444..a248065 100644 --- a/castle/test/headers_filter_test.py +++ b/castle/test/headers_filter_test.py @@ -4,15 +4,15 @@ def headers(): return { - 'Action-Dispatch.request.content-Type': 'application/json', - 'HTTP_AUTHORIZATION': 'Basic 123456', - 'HTTP_COOKIE': "__cid=abcd;other=efgh", - 'HTTP_OK': 'OK', - 'HTTP_ACCEPT': 'application/json', - 'HTTP_X_FORWARDED_FOR': '1.2.3.4', - 'HTTP_USER_AGENT': 'Mozilla 1234', - 'TEST': '1', - 'REMOTE_ADDR': '1.2.3.4' + 'Action-Dispatch.request.content-Type': 'application/json', + 'HTTP_AUTHORIZATION': 'Basic 123456', + 'HTTP_COOKIE': "__cid=abcd;other=efgh", + 'HTTP_OK': 'OK', + 'HTTP_ACCEPT': 'application/json', + 'HTTP_X_FORWARDED_FOR': '1.2.3.4', + 'HTTP_USER_AGENT': 'Mozilla 1234', + 'TEST': '1', + 'REMOTE_ADDR': '1.2.3.4' } @@ -20,12 +20,12 @@ class ExtractorsHeadersTestCase(unittest.TestCase): def test_filter_headers(self): self.assertEqual(HeadersFilter(headers()).call(), { - 'Accept': 'application/json', - 'Authorization': 'Basic 123456', - 'Cookie': "__cid=abcd;other=efgh", - 'Content-Length': '0', - 'Ok': 'OK', - 'User-Agent': 'Mozilla 1234', - 'Remote-Addr': '1.2.3.4', - 'X-Forwarded-For': '1.2.3.4' - }) + 'Accept': 'application/json', + 'Authorization': 'Basic 123456', + 'Cookie': "__cid=abcd;other=efgh", + 'Content-Length': '0', + 'Ok': 'OK', + 'User-Agent': 'Mozilla 1234', + 'Remote-Addr': '1.2.3.4', + 'X-Forwarded-For': '1.2.3.4' + }) diff --git a/castle/test/request_test.py b/castle/test/request_test.py index 7a208e7..98f6e56 100644 --- a/castle/test/request_test.py +++ b/castle/test/request_test.py @@ -1,4 +1,3 @@ -import sys import threading from requests import Response import responses @@ -11,6 +10,7 @@ except ImportError: from http.server import BaseHTTPRequestHandler, HTTPServer + def run_server(): class SimpleHandler(BaseHTTPRequestHandler): def do_POST(self): @@ -21,7 +21,6 @@ def do_POST(self): self.end_headers() self.wfile.write(body) - server = HTTPServer(('', 65521), SimpleHandler) httpd_thread = threading.Thread(target=server.serve_forever) httpd_thread.setDaemon(True) @@ -68,7 +67,6 @@ def test_connection_pooled(self): configuration.port = 443 self.assertEqual(num_pools, 1) - def test_build_url(self): self.assertEqual( Request().build_url('authenticate'), diff --git a/pylintrc b/pylintrc index 0263fb1..0c98018 100644 --- a/pylintrc +++ b/pylintrc @@ -1,10 +1,11 @@ [BASIC] good-names=a,b,i,j,k,ex,Run,_ function-rgx=[a-z_][a-z0-9_]{2,60}$ -method-rgx=[a-z_][a-z0-9_]{2,60}$ +method-rgx=[a-z_][a-zA-Z0-9_]{2,60}$ +class-rgx=[A-Z_][_a-zA-Z0-9]+$ [TYPECHECK] ignored-modules = responses,requests [MESSAGES CONTROL] -disable=missing-docstring,too-many-instance-attributes,attribute-defined-outside-init,too-few-public-methods,dangerous-default-value,duplicate-code,bad-continuation +disable=missing-docstring,too-many-instance-attributes,attribute-defined-outside-init,too-few-public-methods,dangerous-default-value,duplicate-code,bad-continuation,useless-object-inheritance,too-many-public-methods From 8070352da6a3c90537aeb288139f1826388af7d0 Mon Sep 17 00:00:00 2001 From: bartes Date: Tue, 24 Mar 2020 19:50:35 +0100 Subject: [PATCH 8/8] changelog update --- HISTORY.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index d4c2c7c..8dc0d15 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,9 @@ ## master +- [#61](https://github.com/castle/castle-python/pull/61) improve headers and ip extractions, improve ip_headers config, add trusted proxies config, added more events to events list + +https://github.com/castle/castle-python/pull/61 + ## 3.0.0 (2020-02-13) - [#59](https://github.com/castle/castle-python/pull/59) drop requests min version in ci @@ -12,7 +16,7 @@ ## 2.4.0 (2019-11-20) -- [#53](https://github.com/castle/castle-python/pull/53) Update whitelisting and blacklisting behavior +- [#53](https://github.com/castle/castle-python/pull/53) update whitelisting and blacklisting behavior ## 2.3.1 (2019-04-05)