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

zxkeyy · 2024-12-05T20:53:20Z

The problem:
Some specific use cases require inactive users to be able to authenticate, but that is prevented by the code even though django allows it with 'AllowAllUsersModelBackend'.

The changes:
Added new authentication class and authentication rule that allow for inactive users
Added tests for them as well

So instead of having to rewrite the code allowing inactive users can be done by adding these settings:

SIMPLE_JWT = {
      "USER_AUTHENTICATION_RULE": "rest_framework_simplejwt.authentication.allow_inactive_users_authentication_rule",
}

REST_FRAMEWORK = {
    'DEFAULT_AUTHENTICATION_CLASSES': [
        'rest_framework_simplejwt.authentication.JWTInactiveUserAuthentication',
    ]
}

AUTHENTICATION_BACKENDS = [
    'django.contrib.auth.backends.AllowAllUsersModelBackend',
]

closes #607

…active user authentication

…ation rules

for more information, see https://pre-commit.ci

Andrew-Chen-Wang · 2024-12-06T17:28:55Z

Thanks for reaching out! So the offending line of code in the original authentication backend is:

        if not user.is_active:
            raise AuthenticationFailed(_("User is inactive"), code="user_inactive")

I would much prefer if we made a setting called "skip check is_active" where the setting defaults to False (as in by default check is_active) because I can also see how some users may not create an is_active flag. This would knock out two use caes. Ensure the default authentication rule function also checks for this setting.

Update default_user_authentication_rule to use the setting, Update JWTAuthentication.get_user() to test user against authenrication rule instead of is_active flag

for more information, see https://pre-commit.ci

zxkeyy · 2024-12-07T17:23:21Z

@Andrew-Chen-Wang
Updated based on what you suggested, i also changed the check for user.is_active with the user authentication rule since it seems limiting for it to be hardcoded, i still need someone smarter to tell me if this breaks backwards compatibility or something.

Andrew-Chen-Wang

Almost ready, just a few changes

Andrew-Chen-Wang · 2024-12-08T02:15:19Z

rest_framework_simplejwt/authentication.py

@@ -164,6 +165,37 @@ def get_user(self, validated_token: Token) -> AuthUser:
        return api_settings.TOKEN_USER_CLASS(validated_token)


+class JWTInactiveUserAuthentication(JWTAuthentication):


This class shouldn't be needed given the new settings

oops, i missed it. should be good now

Andrew-Chen-Wang · 2024-12-08T02:16:04Z

rest_framework_simplejwt/authentication.py

@@ -131,7 +131,8 @@ def get_user(self, validated_token: Token) -> AuthUser:
        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):


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

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

Andrew-Chen-Wang

thank you! this looks great

zxkeyy and others added 3 commits December 5, 2024 18:55

Feat: Add authentication class and authentication rule that allows in…

14bc650

…active user authentication

Test: Add tests for inactive user authentication and serializer valid…

731d489

…ation rules

[pre-commit.ci] auto fixes from pre-commit.com hooks

6d22c21

for more information, see https://pre-commit.ci

zxkeyy and others added 4 commits December 7, 2024 18:12

Add check if user is active setting,

df7c68a

Update default_user_authentication_rule to use the setting, Update JWTAuthentication.get_user() to test user against authenrication rule instead of is_active flag

Add test for retrieving inactive user in JWT authentication

7789c87

Use new setting in serializer test

adefffe

[pre-commit.ci] auto fixes from pre-commit.com hooks

18f62dc

for more information, see https://pre-commit.ci

Andrew-Chen-Wang requested changes Dec 8, 2024

View reviewed changes

Update get_user to use new setting and remove unnecessary class

8ab5c92

zxkeyy requested a review from Andrew-Chen-Wang December 11, 2024 15:20

Andrew-Chen-Wang approved these changes Dec 11, 2024

View reviewed changes

Andrew-Chen-Wang merged commit 9911c85 into jazzband:master Dec 23, 2024
33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

zxkeyy commented Dec 5, 2024

Andrew-Chen-Wang commented Dec 6, 2024

zxkeyy commented Dec 7, 2024 •

edited

Loading

Andrew-Chen-Wang left a comment

Andrew-Chen-Wang Dec 8, 2024

zxkeyy Dec 8, 2024

Andrew-Chen-Wang Dec 8, 2024 •

edited

Loading

zxkeyy Dec 8, 2024

Andrew-Chen-Wang left a comment

		@@ -164,6 +165,37 @@ def get_user(self, validated_token: Token) -> AuthUser:
		return api_settings.TOKEN_USER_CLASS(validated_token)


		class JWTInactiveUserAuthentication(JWTAuthentication):

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

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

Conversation

zxkeyy commented Dec 5, 2024

Andrew-Chen-Wang commented Dec 6, 2024

zxkeyy commented Dec 7, 2024 • edited Loading

Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

Andrew-Chen-Wang Dec 8, 2024

Choose a reason for hiding this comment

zxkeyy Dec 8, 2024

Choose a reason for hiding this comment

Andrew-Chen-Wang Dec 8, 2024 • edited Loading

Choose a reason for hiding this comment

zxkeyy Dec 8, 2024

Choose a reason for hiding this comment

Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

zxkeyy commented Dec 7, 2024 •

edited

Loading

Andrew-Chen-Wang Dec 8, 2024 •

edited

Loading