Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to allow inactive user authentication and token generation #834

38 changes: 36 additions & 2 deletions rest_framework_simplejwt/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@
except self.user_model.DoesNotExist:
raise AuthenticationFailed(_("User not found"), code="user_not_found")

if not user.is_active:
# Ensure authentication rule passes
if not api_settings.USER_AUTHENTICATION_RULE(user):
Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang Dec 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we want to only validate the user is active, not run the authentication rule, since this function is just looking to fetch a given user. Good forward thinking, but yes this would break backwards compatibility

We can revisit this if you open a GitHub issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about it harder i see how it could cause a problem if a user made their own rule, addressed in latest commit.

raise AuthenticationFailed(_("User is inactive"), code="user_inactive")

if api_settings.CHECK_REVOKE_TOKEN:
Expand Down Expand Up @@ -164,6 +165,37 @@
return api_settings.TOKEN_USER_CLASS(validated_token)


class JWTInactiveUserAuthentication(JWTAuthentication):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class shouldn't be needed given the new settings

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, i missed it. should be good now

"""
An authentication plugin that authenticates requests through a JSON web
token provided in a request header, allowing inactive users to authenticate.
"""

def get_user(self, validated_token: Token) -> AuthUser:
"""
Attempts to find and return a user using the given validated token.
"""
try:
user_id = validated_token[api_settings.USER_ID_CLAIM]
except KeyError:
raise InvalidToken(_("Token contained no recognizable user identification"))

try:
user = self.user_model.objects.get(**{api_settings.USER_ID_FIELD: user_id})
except self.user_model.DoesNotExist:
raise AuthenticationFailed(_("User not found"), code="user_not_found")

if api_settings.CHECK_REVOKE_TOKEN:
if validated_token.get(

Check warning on line 189 in rest_framework_simplejwt/authentication.py

View check run for this annotation

Codecov / codecov/patch

rest_framework_simplejwt/authentication.py#L189

Added line #L189 was not covered by tests
api_settings.REVOKE_TOKEN_CLAIM
) != get_md5_hash_password(user.password):
raise AuthenticationFailed(

Check warning on line 192 in rest_framework_simplejwt/authentication.py

View check run for this annotation

Codecov / codecov/patch

rest_framework_simplejwt/authentication.py#L192

Added line #L192 was not covered by tests
_("The user's password has been changed."), code="password_changed"
)

return user


JWTTokenUserAuthentication = JWTStatelessUserAuthentication


Expand All @@ -175,4 +207,6 @@
# `AllowAllUsersModelBackend`. However, we explicitly prevent inactive
# users from authenticating to enforce a reasonable policy and provide
# sensible backwards compatibility with older Django versions.
return user is not None and user.is_active
return user is not None and (
not api_settings.CHECK_USER_IS_ACTIVE or user.is_active
)
1 change: 1 addition & 0 deletions rest_framework_simplejwt/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"SLIDING_TOKEN_REFRESH_SERIALIZER": "rest_framework_simplejwt.serializers.TokenRefreshSlidingSerializer",
"CHECK_REVOKE_TOKEN": False,
"REVOKE_TOKEN_CLAIM": "hash_password",
"CHECK_USER_IS_ACTIVE": True,
}

IMPORT_STRINGS = (
Expand Down
52 changes: 52 additions & 0 deletions tests/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,31 @@ def test_get_user(self):
# Otherwise, should return correct user
self.assertEqual(self.backend.get_user(payload).id, u.id)

@override_api_settings(
CHECK_USER_IS_ACTIVE=False,
)
def test_get_inactive_user(self):
payload = {"some_other_id": "foo"}

# Should raise error if no recognizable user identification
with self.assertRaises(InvalidToken):
self.backend.get_user(payload)

payload[api_settings.USER_ID_CLAIM] = 42

# Should raise exception if user not found
with self.assertRaises(AuthenticationFailed):
self.backend.get_user(payload)

u = User.objects.create_user(username="markhamill")
u.is_active = False
u.save()

payload[api_settings.USER_ID_CLAIM] = getattr(u, api_settings.USER_ID_FIELD)

# should return correct user
self.assertEqual(self.backend.get_user(payload).id, u.id)

@override_api_settings(
CHECK_REVOKE_TOKEN=True, REVOKE_TOKEN_CLAIM="revoke_token_claim"
)
Expand Down Expand Up @@ -241,3 +266,30 @@ def username(self):

# Restore default TokenUser for future tests
api_settings.TOKEN_USER_CLASS = temp


class TestJWTInactiveUserAuthentication(TestCase):
def setUp(self):
self.backend = authentication.JWTInactiveUserAuthentication()

def test_get_user(self):
payload = {"some_other_id": "foo"}

# Should raise error if no recognizable user identification
with self.assertRaises(InvalidToken):
self.backend.get_user(payload)

payload[api_settings.USER_ID_CLAIM] = 42

# Should raise exception if user not found
with self.assertRaises(AuthenticationFailed):
self.backend.get_user(payload)

u = User.objects.create_user(username="markhamill")
u.is_active = False
u.save()

payload[api_settings.USER_ID_CLAIM] = getattr(u, api_settings.USER_ID_FIELD)

# Otherwise, should return correct user
self.assertEqual(self.backend.get_user(payload).id, u.id)
25 changes: 24 additions & 1 deletion tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.conf import settings
from django.contrib.auth import get_user_model
from django.core import exceptions as django_exceptions
from django.test import TestCase
from django.test import TestCase, override_settings
from rest_framework import exceptions as drf_exceptions

from rest_framework_simplejwt.exceptions import TokenError
Expand Down Expand Up @@ -105,6 +105,29 @@ def test_it_should_raise_if_user_not_active(self):
with self.assertRaises(drf_exceptions.AuthenticationFailed):
s.is_valid()

@override_settings(
AUTHENTICATION_BACKENDS=[
"django.contrib.auth.backends.AllowAllUsersModelBackend",
"django.contrib.auth.backends.ModelBackend",
]
)
@override_api_settings(
CHECK_USER_IS_ACTIVE=False,
)
def test_it_should_validate_if_user_inactive_but_rule_allows(self):
self.user.is_active = False
self.user.save()

s = TokenObtainSerializer(
context=MagicMock(),
data={
TokenObtainSerializer.username_field: self.username,
"password": self.password,
},
)

self.assertTrue(s.is_valid())


class TestTokenObtainSlidingSerializer(TestCase):
def setUp(self):
Expand Down
Loading