From 737ebaa4e9cc865563f5aebfa31902172cdf11bf Mon Sep 17 00:00:00 2001 From: Alexander Dusenbery Date: Mon, 19 Aug 2024 09:38:31 -0400 Subject: [PATCH] feat: optionally ignore ``jwt.exceptions.InvalidTokenErrors`` when decoding JWT cookie Also, `get_decoded_jwt_from_auth()` is now evaluated first in `utils.get_decoded_jwt()`. --- CHANGELOG.rst | 4 ++++ Makefile | 6 +++++- edx_rbac/__init__.py | 2 +- edx_rbac/constants.py | 16 ++++++++++++++++ edx_rbac/models.py | 2 +- edx_rbac/utils.py | 24 +++++++++++++++++++++--- tests/test_utils.py | 35 ++++++++++++++++++++++++++++++----- 7 files changed, 78 insertions(+), 11 deletions(-) create mode 100644 edx_rbac/constants.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ad465e7..1105901 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,10 @@ Change Log Unreleased -------------------- +[1.10.0] +------- +* Optionally ignore and log ``jwt.exceptions.InvalidTokenErrors`` when decoding JWT from cookie. + [1.9.0] ------- * Add support for Python 3.11 & 3.12 diff --git a/Makefile b/Makefile index 6504dc9..d4c11f0 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,7 @@ .PHONY: clean compile_translations coverage diff_cover docs dummy_translations \ extract_translations fake_translations help pii_check pull_translations push_translations \ - quality requirements selfcheck test test-all upgrade validate check_keywords + quality requirements selfcheck test test-all upgrade validate check_keywords \ + virtualenv virtualenv-activate .DEFAULT_GOAL := help @@ -65,6 +66,9 @@ upgrade: $(COMMON_CONSTRAINTS_TXT) ## update the requirements/*.txt files with sed '/^[dD]jango==/d' requirements/test.txt > requirements/test.tmp mv requirements/test.tmp requirements/test.txt +virtualenv: + virtualenv venvs/edx-rbac + quality: ## check coding style with pycodestyle and pylint tox -e quality diff --git a/edx_rbac/__init__.py b/edx_rbac/__init__.py index 7489804..03d7e32 100644 --- a/edx_rbac/__init__.py +++ b/edx_rbac/__init__.py @@ -2,4 +2,4 @@ Library to help managing role based access controls for django apps. """ -__version__ = '1.9.0' +__version__ = '1.10.0' diff --git a/edx_rbac/constants.py b/edx_rbac/constants.py new file mode 100644 index 0000000..246e0f7 --- /dev/null +++ b/edx_rbac/constants.py @@ -0,0 +1,16 @@ +""" +All constants pertaining to the edx_rbac module. +""" +# The string denoting that a given role is applicable across all contexts +ALL_ACCESS_CONTEXT = '*' + +# .. toggle_name: RBAC_IGNORE_INVALID_JWT_COOKIE +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: When true, causes instances of `jwt.exceptions.InvalidTokenError` +# to be ignored instead of raised in the `get_decoded_jwt()` utility function. +# Defaults to False for backward-compatibility purposes, but it's recommended +# to be set to True in dependent applications. +# .. toggle_use_cases: opt_in +# .. toggle_creation_date: 2024-08-19 +IGNORE_INVALID_JWT_COOKIE_SETTING = 'RBAC_IGNORE_INVALID_JWT_COOKIE' diff --git a/edx_rbac/models.py b/edx_rbac/models.py index 738d5e6..99108b7 100644 --- a/edx_rbac/models.py +++ b/edx_rbac/models.py @@ -15,7 +15,7 @@ class UserRoleAssignmentCreator(ModelBase): The model extending UserRoleAssignment should get a foreign key to a model that is a subclass of UserRole. """ - def __new__(mcs, name, bases, attrs): # pylint: disable=arguments-differ + def __new__(mcs, name, bases, attrs): """ Override to dynamically create foreign key for objects begotten from abstract class. """ diff --git a/edx_rbac/utils.py b/edx_rbac/utils.py index f749594..8665749 100644 --- a/edx_rbac/utils.py +++ b/edx_rbac/utils.py @@ -5,13 +5,17 @@ import importlib from collections import OrderedDict, defaultdict from collections.abc import Iterable +from logging import getLogger from django.apps import apps from django.conf import settings from edx_rest_framework_extensions.auth.jwt.authentication import get_decoded_jwt_from_auth from edx_rest_framework_extensions.auth.jwt.cookies import get_decoded_jwt as get_decoded_jwt_from_cookie +from jwt.exceptions import InvalidTokenError -ALL_ACCESS_CONTEXT = '*' +from edx_rbac.constants import ALL_ACCESS_CONTEXT, IGNORE_INVALID_JWT_COOKIE_SETTING + +logger = getLogger(__name__) def request_user_has_implicit_access_via_jwt(decoded_jwt, role_name, context=None): @@ -203,8 +207,22 @@ def get_decoded_jwt(request): Decodes the request's JWT from either cookies or auth payload and returns it. Defaults to an empty dictionary. """ - decoded_jwt = get_decoded_jwt_from_cookie(request) or get_decoded_jwt_from_auth(request) - return decoded_jwt or {} + if decoded_jwt_from_auth := get_decoded_jwt_from_auth(request): + return decoded_jwt_from_auth + + try: + if decoded_jwt_from_cookie := get_decoded_jwt_from_cookie(request): + return decoded_jwt_from_cookie + except InvalidTokenError as exc: + logger.info( + '[edx_rbac.get_decoded_jwt] Error decoding JWT from cookie and no JWT found in auth: %s', + exc, + ) + # Django settings toggle: see toggle annotation in `constants.py`. + if not getattr(settings, IGNORE_INVALID_JWT_COOKIE_SETTING, False): + raise + + return {} def has_access_to_all(assigned_contexts): diff --git a/tests/test_utils.py b/tests/test_utils.py index bace0b5..422418b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -8,10 +8,11 @@ import ddt from django.contrib import auth from django.contrib.auth.models import AnonymousUser -from django.test import RequestFactory, TestCase +from django.test import RequestFactory, TestCase, override_settings +from jwt.exceptions import InvalidTokenError +from edx_rbac.constants import ALL_ACCESS_CONTEXT, IGNORE_INVALID_JWT_COOKIE_SETTING from edx_rbac.utils import ( - ALL_ACCESS_CONTEXT, _user_has_access, contexts_accessible_from_request, create_role_auth_claim_for_user, @@ -347,15 +348,15 @@ def test_is_iterable(self, a_function, expected_value): def test_set_from_collection_or_single_item(self, obj, expected_value): assert expected_value == set_from_collection_or_single_item(obj) - @mock.patch('edx_rbac.utils.get_decoded_jwt_from_auth') + @mock.patch('edx_rbac.utils.get_decoded_jwt_from_auth', return_value=None) @mock.patch('edx_rbac.utils.get_decoded_jwt_from_cookie') def test_get_decoded_jwt_when_it_exists_in_cookie(self, mock_jwt_from_cookie, mock_jwt_from_auth): request = mock.Mock() assert mock_jwt_from_cookie.return_value == get_decoded_jwt(request) + mock_jwt_from_auth.assert_called_once_with(request) mock_jwt_from_cookie.assert_called_once_with(request) - self.assertFalse(mock_jwt_from_auth.called) @mock.patch('edx_rbac.utils.get_decoded_jwt_from_auth') @mock.patch('edx_rbac.utils.get_decoded_jwt_from_cookie', return_value=None) @@ -364,7 +365,7 @@ def test_get_decoded_jwt_when_it_exists_only_in_auth(self, mock_jwt_from_cookie, assert mock_jwt_from_auth.return_value == get_decoded_jwt(request) - mock_jwt_from_cookie.assert_called_once_with(request) + self.assertFalse(mock_jwt_from_cookie.called) mock_jwt_from_auth.assert_called_once_with(request) @mock.patch('edx_rbac.utils.get_decoded_jwt_from_auth', return_value=None) @@ -377,6 +378,30 @@ def test_get_decoded_jwt_defaults_to_empty_dict(self, mock_jwt_from_cookie, mock mock_jwt_from_cookie.assert_called_once_with(request) mock_jwt_from_auth.assert_called_once_with(request) + @override_settings(**{IGNORE_INVALID_JWT_COOKIE_SETTING: True}) + @mock.patch('edx_rbac.utils.get_decoded_jwt_from_auth', return_value=None) + @mock.patch('edx_rbac.utils.get_decoded_jwt_from_cookie') + def test_get_decoded_jwt_ignores_invalid_token_errors(self, mock_jwt_from_cookie, mock_jwt_from_auth): + request = mock.Mock() + mock_jwt_from_cookie.side_effect = InvalidTokenError('foo') + + assert {} == get_decoded_jwt(request) + + mock_jwt_from_cookie.assert_called_once_with(request) + mock_jwt_from_auth.assert_called_once_with(request) + + @mock.patch('edx_rbac.utils.get_decoded_jwt_from_auth', return_value=None) + @mock.patch('edx_rbac.utils.get_decoded_jwt_from_cookie') + def test_get_decoded_jwt_raises_invalid_token_errors_by_default(self, mock_jwt_from_cookie, mock_jwt_from_auth): + request = mock.Mock() + mock_jwt_from_cookie.side_effect = InvalidTokenError('foo') + + with self.assertRaises(InvalidTokenError): + get_decoded_jwt(request) + + mock_jwt_from_cookie.assert_called_once_with(request) + mock_jwt_from_auth.assert_called_once_with(request) + @ddt.data( ({}, False), ({ALL_ACCESS_CONTEXT}, True),