diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ac03a44..72c53e8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,10 +6,10 @@ on: [push] jobs: lint: - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 with: python-version: '3.x' - name: Install requirements @@ -25,12 +25,12 @@ jobs: fail-fast: true name: CKAN ${{ matrix.ckan-version }} - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest container: image: openknowledge/ckan-dev:${{ matrix.ckan-version }} steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Install requirements run: | diff --git a/README.md b/README.md index f6748b6..3096e6e 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,11 @@ replace the FriendlyForm plugin in `who.ini` with a token-aware version: use = ckanext.csrf_filter.token_protected_friendlyform:TokenProtectedFriendlyFormPlugin ``` +1. Optional: To set token cookie [SameSite attribute](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#samesitesamesite-value), set ``ckanext.csrf_filter.same_site`` setting in your CKAN config file. By default, the SameSite attribute will be ``None``. Supported values: + * Strict + * Lax + * None + 1. Restart CKAN. Eg if you've deployed CKAN with Apache on Ubuntu: ``` @@ -91,6 +96,19 @@ Optional # Default 10 minutes. ckanext.csrf_filter.token_rotation_minutes = 10 + # Exempts given regex matches from token checks. + # Default None. + # Must be in a parsable JSON list format. + # Strings must be double quoted. + # WARNING: this is a very powerful feature. Please make sure that your regex rules are strict. + ckanext.csrf_filter.exempt_rules = [ + "^/do/not/check/this/path/.*", + "^/datatables/ajax/.*" + ] + + # Cookie samesite attribute value. Defaults to None + ckanext.csrf_filter.same_site = Strict + Testing ======= diff --git a/ckanext/csrf_filter/anti_csrf.py b/ckanext/csrf_filter/anti_csrf.py index d24942a..5eeabf1 100644 --- a/ckanext/csrf_filter/anti_csrf.py +++ b/ckanext/csrf_filter/anti_csrf.py @@ -11,6 +11,7 @@ import hashlib import hmac +import json from logging import getLogger import random import re @@ -43,7 +44,6 @@ ENCODED_TOKEN_VALIDATION_PATTERN = re.compile( r'^[0-9a-z]+![0-9]+/[0-9]+/[-_a-z0-9%+=]+$', re.IGNORECASE) -API_URL = re.compile(r'^/+api/.*') LOGIN_URL = re.compile(r'^(/user)?/log(ged_)?in(_generic)?') CONFIRM_MODULE_PATTERN = r'data-module=["\']confirm-action["\']' CONFIRM_MODULE = re.compile(CONFIRM_MODULE_PATTERN) @@ -64,9 +64,11 @@ def configure(config): """ Configure global values for the filter. """ global secure_cookies + global same_site global secret_key global token_expiry_age global token_renewal_age + global exempt_rules site_url = urlparse(config.get('ckan.site_url', '')) if site_url.scheme == 'https': @@ -75,6 +77,10 @@ def configure(config): LOG.warning("Site %s is not secure! CSRF tokens may be exposed!", site_url) secure_cookies = False + same_site = config.get('ckanext.csrf_filter.same_site', 'None') + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#samesitesamesite-value + assert same_site in ['Strict', 'Lax', 'None'] + key_fields = ['ckanext.csrf_filter.secret_key', 'beaker.session.secret', 'flask.secret_key'] @@ -90,6 +96,13 @@ def configure(config): token_expiry_age = 60 * config.get('ckanext.csrf_filter.token_expiry_minutes', 30) token_renewal_age = 60 * config.get('ckanext.csrf_filter.token_renewal_minutes', 10) + exempt_rules = [re.compile(r'^/+api/.*')] + custom_exempt_rules = config.get('ckanext.csrf_filter.exempt_rules', None) + if custom_exempt_rules: + for rule in json.loads(custom_exempt_rules): + LOG.debug("Adding CSRF exclusion: %s", rule) + exempt_rules.append(re.compile(rule)) + # ------------- # Token parsing @@ -204,8 +217,10 @@ def _is_request_exempt(request): as are API calls (which should instead provide an API key). """ request_helper = RequestHelper(request) + for rule in exempt_rules: + if rule.match(request_helper.get_path()): + return True return not is_logged_in(request) \ - or API_URL.match(request_helper.get_path()) \ or request_helper.get_method() in {'GET', 'HEAD', 'OPTIONS'} @@ -311,7 +326,7 @@ def _get_digest(message): def _set_response_token_cookie(token, response): """ Add a generated token cookie to the HTTP response. """ - response.set_cookie(TOKEN_FIELD_NAME, token, secure=secure_cookies, httponly=True) + response.set_cookie(TOKEN_FIELD_NAME, token, secure=secure_cookies, httponly=True, samesite=same_site) def create_response_token(): diff --git a/ckanext/csrf_filter/helpers.py b/ckanext/csrf_filter/helpers.py new file mode 100644 index 0000000..7da0b3e --- /dev/null +++ b/ckanext/csrf_filter/helpers.py @@ -0,0 +1,18 @@ +from markupsafe import Markup +from ckanext.csrf_filter.anti_csrf import get_response_token, TOKEN_FIELD_NAME + +try: + from ckan.common import is_flask_request +except ImportError: + def is_flask_request(): + return True + + +def csrf_token_field(): + if is_flask_request(): + from flask import Response + response = Response() + else: + from pylons import response + token = get_response_token(response) + return Markup(''.format(TOKEN_FIELD_NAME, token)) diff --git a/ckanext/csrf_filter/plugin.py b/ckanext/csrf_filter/plugin.py index cf555a2..0f9c65a 100644 --- a/ckanext/csrf_filter/plugin.py +++ b/ckanext/csrf_filter/plugin.py @@ -3,11 +3,13 @@ """ from logging import getLogger +from types import GeneratorType from ckan import plugins from ckan.plugins import implements, toolkit from ckanext.csrf_filter import anti_csrf +import ckanext.csrf_filter.helpers as h from ckanext.csrf_filter.request_helpers import RequestHelper @@ -39,14 +41,21 @@ class CSRFFilterPlugin(plugins.SingletonPlugin): """ Inject CSRF tokens into HTML responses, and validate them on applicable requests. """ + implements(plugins.IConfigurer) implements(plugins.IConfigurable, inherit=True) implements(plugins.IAuthenticator, inherit=True) + implements(plugins.ITemplateHelpers) if not toolkit.check_ckan_version('2.9'): implements(plugins.IRoutes, inherit=True) if toolkit.check_ckan_version(min_version='2.8.0'): implements(plugins.IBlueprint, inherit=True) implements(plugins.IMiddleware, inherit=True) + # IConfigurer + + def update_config(self, config_): + toolkit.add_template_directory(config_, 'templates') + # IConfigurable def configure(self, config): @@ -62,6 +71,11 @@ def login(self): request.get_environ()['__no_cache__'] = True return None + # ITemplateHelpers + + def get_helpers(self): + return {'csrf_token_field': h.csrf_token_field} + # IRoutes def after_map(self, route_map): @@ -94,7 +108,16 @@ def check_csrf(): @blueprint.after_app_request def set_csrf_token(response): """ Apply a CSRF token to all response bodies. + + Exclude GeneratorType responses as they are data streams. + Modifying the data of the data stream breaks the streaming process. + + If a user needs to stream templates, they should use the csrf_token_field + helper in their forms inside of their streamed templates. """ + if isinstance(getattr(response, 'response', None), GeneratorType): + return response + response.direct_passthrough = False anti_csrf.apply_token(response) return response diff --git a/ckanext/csrf_filter/templates/user/snippets/api_token_list.html b/ckanext/csrf_filter/templates/user/snippets/api_token_list.html new file mode 100644 index 0000000..5e3c64f --- /dev/null +++ b/ckanext/csrf_filter/templates/user/snippets/api_token_list.html @@ -0,0 +1,15 @@ +{% ckan_extends %} + +{% block token_cell_actions %} + + {% set action = h.url_for("user.api_token_revoke", id=user['name'], jti=token['id']) %} +
+ {{ h.csrf_input() }} +
+ +
+
+ +{% endblock token_cell_actions %} diff --git a/ckanext/csrf_filter/test_anti_csrf.py b/ckanext/csrf_filter/test_anti_csrf.py index 92b8e32..ed26ce0 100644 --- a/ckanext/csrf_filter/test_anti_csrf.py +++ b/ckanext/csrf_filter/test_anti_csrf.py @@ -7,6 +7,12 @@ import six import unittest +try: + from json import JSONDecodeError +except ImportError: + # Python 2 JSON library raises ValueError instead + JSONDecodeError = ValueError + from ckanext.csrf_filter import anti_csrf try: @@ -70,7 +76,8 @@ def mock_objects(username=None): """ anti_csrf.configure({ 'ckanext.csrf_filter.secret_key': 'secret', - 'ckan.site_url': 'https://unit-test'}) + 'ckan.site_url': 'https://unit-test', + 'ckanext.csrf_filter.exempt_rules': '["^/datatables/ajax/.*"]'}) if username: anti_csrf._get_user = lambda: MockUser(username) else: @@ -248,6 +255,41 @@ def _check_config(self, secret_key, secure_cookies=False, self.assertEqual(anti_csrf.token_expiry_age, token_expiry_age) self.assertEqual(anti_csrf.token_renewal_age, token_renewal_age) + def test_exempt_rules(self): + """ Tests that requests matching the exemption rules are not checked for tokens + """ + mock_objects('unit-test') + + # tests exempt rule skips token check + path = '/datatables/ajax/331fed84-2c8d-4d2e-b9ee-9ce8cbda3352' + request = MockRequest(method='', path=path, cookies={'auth_tkt': 'unit-test'}) + print("Expecting check_csrf to pass for {}".format(path)) + self.assertTrue(anti_csrf.check_csrf(request)) + + path = '/datatables/filtered-download/331fed84-2c8d-4d2e-b9ee-9ce8cbda3352' + request = MockRequest(method='', path=path, cookies={'auth_tkt': 'unit-test'}) + print("Expecting check_csrf to fail for {}".format(path)) + self.assertFalse(anti_csrf.check_csrf(request)) + + # test multiple regex rules + config = {'ckanext.csrf_filter.secret_key': 'secret_key'} + config['ckanext.csrf_filter.exempt_rules'] = '["^/datatables/ajax/.*", "/datatables/filtered-download/.*"]' + expected = [ + '^/+api/.*', + '^/datatables/ajax/.*', + '/datatables/filtered-download/.*' + ] + anti_csrf.configure(config) + # Use custom matching since equivalent patterns won't necessarily + # compile to equal objects under all Python versions + self.assertEqual(len(anti_csrf.exempt_rules), len(expected)) + for index in range(len(anti_csrf.exempt_rules)): + self.assertEquals(anti_csrf.exempt_rules[index].pattern, expected[index]) + + # test bad JSON string + config['ckanext.csrf_filter.exempt_rules'] = '^/datatables/ajax/.*", "/datatables/filtered-download/.*' + self.assertRaises(JSONDecodeError, anti_csrf.configure, config) + if __name__ == '__main__': unittest.main()