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

JSONWebTokenSerializer.validate no longer fails if the user is not active #98

Open
nigel-gott opened this issue Aug 20, 2021 · 1 comment

Comments

@nigel-gott
Copy link

nigel-gott commented Aug 20, 2021

Hi, I am currently upgrading a project to use this fork from the original jpadilla version. So far the breaking changes have been clearly documented however I ran across this one which was not.

Previously ObtainJSONWebTokenView in jpadilla's version would raise ValidationError("User account is disabled.") if a non active user attempted to use the view and obtain a token. However now in the "Dropped support for drf<3.7, django<1.11. Refactored tests. " commit JSONWebTokenSerializer.validate was changed to no longer fail if the user was inactive.

The other views provided by this library use serializers like VerifyAuthTokenSerializer and RefreshAuthTokenSerializer which call check_user in their validate method which does raise for inactive users, however ObtainJSONWebTokenView uses JSONWebTokenSerializer which no longer does.

We can work around this change in our usage of drf-jwt for now, however:

  1. I'm not sure how intended this change in behaviour was, but perhaps there is a good reason for this change?
  2. It's a bit odd that the other views do check this but ObtainJSONWebTokenView does not.
  3. This is a breaking change from the old version and might trip up other users migrating.

If there is a good reason for this change then I am happy to open an MR updating the documentation to clearly state this change. However if not and we believe this should be fixed then I am also happy to fix it. To do so my initial thoughts are:

  1. Call check_user in JSONWebTokenSerializer.validate
  2. Create a new Serializer for ObtainJSONWebTokenView which calls check_user
  3. Copy the exact old check from jpadilla's JSONWebTokenSerializer.validate back into this libraries version

Let me know which if any of these you would prefer.

Thanks for the fork and all the new features!

@jaap3
Copy link

jaap3 commented Feb 14, 2022

Note that Django can be configured to use AUTHENTICATION_BACKENDS = ['django.contrib.auth.backends.AllowAllUsersModelBackend'], which will allows inactive users to authenticate.

Luckily we caught this in tests, otherwise it would've suddenly be possible to get tokens for inactive users.

As a workaround the is_active flag is now checked in a custom JWT_RESPONSE_PAYLOAD_HANDLER

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants