Skip to content

Commit

Permalink
feat: optionally ignore jwt.exceptions.InvalidTokenErrors when de…
Browse files Browse the repository at this point in the history
…coding JWT cookie

Also, `get_decoded_jwt_from_auth()` is now evaluated first in `utils.get_decoded_jwt()`.
  • Loading branch information
iloveagent57 committed Aug 19, 2024
1 parent 84a561c commit 078fa38
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 11 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion edx_rbac/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Library to help managing role based access controls for django apps.
"""

__version__ = '1.9.0'
__version__ = '1.10.0'
16 changes: 16 additions & 0 deletions edx_rbac/constants.py
Original file line number Diff line number Diff line change
@@ -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'
2 changes: 1 addition & 1 deletion edx_rbac/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down
24 changes: 21 additions & 3 deletions edx_rbac/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
35 changes: 30 additions & 5 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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),
Expand Down

0 comments on commit 078fa38

Please sign in to comment.