jpadilla / django-rest-framework-jwt

JSON Web Token Authentication support for Django REST Framework
http://jpadilla.github.io/django-rest-framework-jwt/
MIT License
3.19k stars 652 forks source link

Fix is_active error message #391

Open iliasgal opened 7 years ago

iliasgal commented 7 years ago

Fix for issue #303

If user is disabled (is_active=False), the authenticate function returns None, so the user variable in:

user = authenticate(**credentials)

is always None.

Therefore, the if statement below is always False and the commands are never executed.

if not user.is_active:
    msg = _('User account is disabled.')
    raise serializers.ValidationError(msg)

My suggestion is to move the check for if not user.is_active: under the else statement.

We get the user with a query based on the given username. We also add an exception if ObjectDoesNotExist.

If user exists, then we check if it is active or not. If the ObjectDoesNotExist is thrown, then it means that the login credentials were not correct.

 try:
    user = User.objects.get(**{self.username_field: attrs.get(self.username_field)})
    if not user.is_active:
        msg = _('User account is disabled.')
        raise serializers.ValidationError(msg)
    except ObjectDoesNotExist:
        msg = _('Unable to log in with provided credentials.')
        raise serializers.ValidationError(msg)
manan commented 6 years ago

Waiting for the merge :)

manan commented 6 years ago

And just FYI, this commit will not have expected behaviour in the case where someone sends a username of an inactive user and an INCORRECT password.

According to the commit, the response will be 'User account is disabled.' but it should be 'Unable to log in with provided credentials.' as the password in itself is incorrect. The way I see it is, the only time 'User account is disabled.' should be returned is when the credentials are correct but the user is inactive.